Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Thierry,

在 2015/8/10 18:00, Thierry Reding 写道:
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
[...]
         edp: edp@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.

I do think so, those numbers can parse from struct drm_mode. But I haven't
send those changes yet, cause I want to merge the split analogix_dp first, and then send some patches to improve it. If you think it's better to imptoved those
now, I would like to do it , please let me know ;)

                 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.

Same to previous reply

                 samsung,color-depth = <1>;
This is probably drm_display_info.bpc.

Same to previous reply

                 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?

Yes, I do meet the problem that my eDP screen need lane-count to 4, but my
DP TV need lane-count to 1. Just like previous reply, if you think I should improved
them in this series, I would rather to do it.

+	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.

The sclk_dp & sclk_dp_24m is not IP common ask, it's only exist in RK3288 SoC (Like exynos only got one "dp" clock), and actually I add this to rockchip platform dp driver not analogix
dp driver. So I think it's okay to add some platform some common prefixes.

And I got a better idea for those clock. "sclk_dp" & "sclk_dp_24m" is provided for the eDP phy, and I just take Heiko suggest that add an new phy-rockchip-dp.c driver, so it's better to move
those clock to phy driver, and rename them to "dp-phy" && "dp-phy-24m".

Thanks,
- Yakir

Thierry


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux