[PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Thierry Reding
treding at nvidia.com
Mon Aug 10 03:00:57 PDT 2015
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
[...]
> edp: edp at ff970000 {
[...]
> hsync-active-high = <0>;
> vsync-active-high = <0>;
> interlaced = <0>;
These look like they should come from the display mode definition (EDID)
rather than device tree.
> samsung,color-space = <0>;
> samsung,dynamic-range = <0>;
> samsung,ycbcr-coeff = <0>;
I think these should also come from EDID, though I'm not sure if we
store this in internal data structures yet.
> samsung,color-depth = <1>;
This is probably drm_display_info.bpc.
> samsung,link-rate = <0x0a>;
> samsung,lane-count = <1>;
And these should really be derived from values in the DPCD and adjusted
(if necessary) during link training.
Why would you ever want to hard-code the above?
> >>+ dp->clk_24m = devm_clk_get(dev, "clk_dp_24m");
> >Same here, maybe "dp_24m".
> Like my previous reply. And actually as those two clocks all have
> a common prefix "SCLK" in rk3288 clock tree, I thinkt we can name
> them to "sclk_dp" & "sclk_dp_24m", is it okay ?
I don't think there's a need for these common prefixes. The names here
are identifiers in the context of the IP block, so any SoC-specific
prefixes are irrelevant. Also they do appear, in DT and in code, in the
context of clocks already, so "sclk_" or "clk_" is completely redundant
in these names.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-rockchip/attachments/20150810/80d87f4c/attachment.sig>
More information about the Linux-rockchip
mailing list