Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Thierry,
在 2015/8/10 21:17, Thierry Reding 写道:
On Mon, Aug 10, 2015 at 08:59:44PM +0800, Yakir Yang wrote:
Hi Thierry,
在 2015/8/10 18:00, Thierry Reding 写道:
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
[...]
edp: edp@ff97 {
[...]
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.
The problem with these is that if you keep them in for your initial
submission, you can never (or only under extreme pain) remove them.
Anything in DTB needs to be effectively supported forever.
Also since these don't make sense to hard-code, just improve the code
and get rid of the need for these DT properties. Mind you that you still
need to keep the code to parse them, because presumably Exynos relies on
them. But depending on how you split up the driver you might be able to
restrict these compatibility hacks to Exynos and not carry them forward
into your new driver.
Okay, thanks for your remind ;)
+ 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".
I agree that dealing with these in a PHY driver sounds like the better
option. However, I still think that the dp-phy prefix is redundant. The
names are in a per-driver scope, so "dp-phy" is implied by the device
tree binding and driver already. You could simply use shorter names such
as "phy" and "24m" for example.
Also note that the clock provider will already have the proper names for
these, so the clock tree will end up showing the provider names. The
names in the binding are merely the "consumer" names.
Agree, thanks
- Yakir
Thierry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
On Mon, Aug 10, 2015 at 08:59:44PM +0800, Yakir Yang wrote:
> Hi Thierry,
>
> 在 2015/8/10 18:00, Thierry Reding 写道:
> >On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
> >[...]
> >> edp: edp@ff97 {
> >[...]
> >> 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.
The problem with these is that if you keep them in for your initial
submission, you can never (or only under extreme pain) remove them.
Anything in DTB needs to be effectively supported forever.
Also since these don't make sense to hard-code, just improve the code
and get rid of the need for these DT properties. Mind you that you still
need to keep the code to parse them, because presumably Exynos relies on
them. But depending on how you split up the driver you might be able to
restrict these compatibility hacks to Exynos and not carry them forward
into your new driver.
> + 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".
I agree that dealing with these in a PHY driver sounds like the better
option. However, I still think that the dp-phy prefix is redundant. The
names are in a per-driver scope, so "dp-phy" is implied by the device
tree binding and driver already. You could simply use shorter names such
as "phy" and "24m" for example.
Also note that the clock provider will already have the proper names for
these, so the clock tree will end up showing the provider names. The
names in the binding are merely the "consumer" names.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Heiko,
在 2015/8/10 20:08, Heiko Stübner 写道:
Hi Yakir,
Am Samstag, 8. August 2015, 11:54:38 schrieb Yakir Yang:
+static int rockchip_dp_init(struct rockchip_dp_device *dp)
+{
+ struct device *dev = dp->dev;
+ struct device_node *np = dev->of_node;
+ int ret;
+
+ dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+ if (IS_ERR(dp->grf)) {
+ dev_err(dev,
+ "rk3288-dp needs rockchip,grf property\n");
+ return PTR_ERR(dp->grf);
+ }
+
+ dp->clk_dp = devm_clk_get(dev, "clk_dp");
I've looked at the manual, but couldn't find an actual clock-name
used there. Is it really "clk_dp" or should it just be "dp"?
This should be "clk_dp", not "dp".
Cause analogix_dp_core would need a clock name with "dp", so I would
rather to pasted my rockchip-dp node here before I add dt-bindings in
next version ;)
The clock we name PCLK_EDP_CTRL in the clock controller is probably the clock
supplying the APB interface and named pclk already in the "Figure 3-2
DP_TXclock domain" diagram on page 19 of the manual. So your "clk_dp" should
actually be "pclk".
So you would have "dp", "dp_24m" and "pclk" for the 3 supplying clocks.
Oh, yes, "pclk" is for APB interface, and "sclk_edp" for IP controller,
and "sclk_edp_24m" for DP PHY,
thanks for your explain.
So for now, I would pass "sclk_edp" to "edp" in analogix_dp, and
"sclk_edp_24m" to "dp-phy_24m"
in phy-rockchip-dp.c, and "pclk_edp" to "pclk" in analogix_dp-rockchip.c.
edp: edp@ff97 {
compatible = "rockchip,rk3288-dp";
reg = <0xff97 0x4000>;
interrupts = ;
clocks = < SCLK_EDP>, < SCLK_EDP_24M>, <
PCLK_EDP_CTRL>;
clock-names = "clk_dp", "clk_dp_24m", "dp";
rockchip,grf = <>;
resets = < 111>;
reset-names = "dp";
power-domains = < RK3288_PD_VIO>;
status = "disabled";
hsync-active-high = <0>;
vsync-active-high = <0>;
interlaced = <0>;
samsung,color-space = <0>;
samsung,dynamic-range = <0>;
samsung,ycbcr-coeff = <0>;
samsung,color-depth = <1>;
samsung,link-rate = <0x0a>;
samsung,lane-count = <1>;
Thierry already said, that these should probably be somehow auto-detected.
Properties needing to stay around should probably also be "analogix,..." with
a fallback to not break Samsung devicetrees, so
look for "analogix,foo!, if not found try "samsung,foo"
Okay, it's better to rename to "analogxi...", done.
ports {
edp_in: port {
#address-cells = <1>;
#size-cells = <0>;
edp_in_vopb: endpoint@0 {
reg = <0>;
remote-endpoint = <_out_edp>;
};
};
};
+
+ 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 ?
As Thierry said, please don't add prefixes.
Okay, so is it okay to rename them to "dp", "dp-phy-24m", "pclk" ?
+ if (IS_ERR(dp->clk_24m)) {
+ dev_err(dev, "cannot get clk_dp_24m\n");
+ return PTR_ERR(dp->clk_24m);
+ }
I think you're missing the pclk here (PCLK_EDP_CTRL) or is this part of
something else?
Whops, as I refered in commit message I leave pclk_dp to
analogix_dp_core driver ;-)
The reason why I want to leave pclk is I thought this clock is more like
analogix dp
core driver want, like a IP controller clock (whatever analogix_dp do
need a clock
named with "dp").
Hmm, I'd think what the core (and Samsung) driver use as "dp" clock is
probably the generic clock for the IP and not the pclk for the APB interface.
So I think it still should be "dp" for the core and "dp_24m" + "pclk" for the
rockchip part?
Yes, I think you are right, thanks ;)
+
+ dp->rst = devm_reset_control_get(dev, "dp");
+ if (IS_ERR(dp->rst)) {
+ dev_err(dev, "failed to get reset\n");
+ return PTR_ERR(dp->rst);
+ }
+
+ ret = rockchip_dp_clk_enable(dp);
+ if (ret < 0) {
+ dev_err(dp->dev, "cannot enable dp clk %d\n", ret);
+ return ret;
+ }
+
+ ret = rockchip_dp_pre_init(dp);
+ if (ret < 0) {
+ dev_err(dp->dev, "failed to pre init %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
[...]
+static int rockchip_dp_probe(struct
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Thierry,
在 2015/8/10 18:00, Thierry Reding 写道:
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
[...]
edp: edp@ff97 {
[...]
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Yakir,
Am Samstag, 8. August 2015, 11:54:38 schrieb Yakir Yang:
> >> +static int rockchip_dp_init(struct rockchip_dp_device *dp)
> >> +{
> >> + struct device *dev = dp->dev;
> >> + struct device_node *np = dev->of_node;
> >> + int ret;
> >> +
> >> + dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> >> + if (IS_ERR(dp->grf)) {
> >> + dev_err(dev,
> >> + "rk3288-dp needs rockchip,grf property\n");
> >> + return PTR_ERR(dp->grf);
> >> + }
> >> +
> >> + dp->clk_dp = devm_clk_get(dev, "clk_dp");
> >
> > I've looked at the manual, but couldn't find an actual clock-name
> > used there. Is it really "clk_dp" or should it just be "dp"?
>
> This should be "clk_dp", not "dp".
> Cause analogix_dp_core would need a clock name with "dp", so I would
> rather to pasted my rockchip-dp node here before I add dt-bindings in
> next version ;)
The clock we name PCLK_EDP_CTRL in the clock controller is probably the clock
supplying the APB interface and named pclk already in the "Figure 3-2
DP_TXclock domain" diagram on page 19 of the manual. So your "clk_dp" should
actually be "pclk".
So you would have "dp", "dp_24m" and "pclk" for the 3 supplying clocks.
>
> edp: edp@ff97 {
> compatible = "rockchip,rk3288-dp";
> reg = <0xff97 0x4000>;
> interrupts = ;
>
> clocks = < SCLK_EDP>, < SCLK_EDP_24M>, <
> PCLK_EDP_CTRL>;
> clock-names = "clk_dp", "clk_dp_24m", "dp";
>
> rockchip,grf = <>;
> resets = < 111>;
> reset-names = "dp";
> power-domains = < RK3288_PD_VIO>;
> status = "disabled";
>
> hsync-active-high = <0>;
> vsync-active-high = <0>;
> interlaced = <0>;
> samsung,color-space = <0>;
> samsung,dynamic-range = <0>;
> samsung,ycbcr-coeff = <0>;
> samsung,color-depth = <1>;
> samsung,link-rate = <0x0a>;
> samsung,lane-count = <1>;
Thierry already said, that these should probably be somehow auto-detected.
Properties needing to stay around should probably also be "analogix,..." with
a fallback to not break Samsung devicetrees, so
look for "analogix,foo!, if not found try "samsung,foo"
> ports {
> edp_in: port {
> #address-cells = <1>;
> #size-cells = <0>;
> edp_in_vopb: endpoint@0 {
> reg = <0>;
> remote-endpoint = <_out_edp>;
> };
> };
> };
> >> +
> >> + 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 ?
As Thierry said, please don't add prefixes.
>
> >> + if (IS_ERR(dp->clk_24m)) {
> >> + dev_err(dev, "cannot get clk_dp_24m\n");
> >> + return PTR_ERR(dp->clk_24m);
> >> + }
> >
> > I think you're missing the pclk here (PCLK_EDP_CTRL) or is this part of
> > something else?
>
> Whops, as I refered in commit message I leave pclk_dp to
> analogix_dp_core driver ;-)
>
> The reason why I want to leave pclk is I thought this clock is more like
> analogix dp
> core driver want, like a IP controller clock (whatever analogix_dp do
> need a clock
> named with "dp").
Hmm, I'd think what the core (and Samsung) driver use as "dp" clock is
probably the generic clock for the IP and not the pclk for the APB interface.
So I think it still should be "dp" for the core and "dp_24m" + "pclk" for the
rockchip part?
>
> >> +
> >> + dp->rst = devm_reset_control_get(dev, "dp");
> >> + if (IS_ERR(dp->rst)) {
> >> + dev_err(dev, "failed to get reset\n");
> >> + return PTR_ERR(dp->rst);
> >> + }
> >> +
> >> + ret = rockchip_dp_clk_enable(dp);
> >> + if (ret < 0) {
> >> + dev_err(dp->dev, "cannot enable dp clk %d\n", ret);
> >> + return ret;
> >> + }
> >> +
> >> + ret = rockchip_dp_pre_init(dp);
> >> + if (ret < 0) {
> >> + dev_err(dp->dev, "failed to pre init %d\n", ret);
> >> + return ret;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >
> > [...]
> >
> >> +static int rockchip_dp_probe(struct platform_device *pdev)
> >> +{
> >> + struct device *dev = >dev;
> >> + struct device_node *panel_node;
> >> + struct rockchip_dp_device *dp;
> >> + struct drm_panel *panel;
> >> +
> >> + panel_node = of_parse_phandle(dev->of_node, "rockchip,panel", 0);
> >> + if (!panel_node) {
> >> + DRM_ERROR("failed to find rockchip,panel dt
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
[...]
> edp: edp@ff97 {
[...]
> 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
signature.asc
Description: PGP signature
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Thierry,
在 2015/8/10 21:17, Thierry Reding 写道:
On Mon, Aug 10, 2015 at 08:59:44PM +0800, Yakir Yang wrote:
Hi Thierry,
在 2015/8/10 18:00, Thierry Reding 写道:
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
[...]
edp: edp@ff97 {
[...]
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.
The problem with these is that if you keep them in for your initial
submission, you can never (or only under extreme pain) remove them.
Anything in DTB needs to be effectively supported forever.
Also since these don't make sense to hard-code, just improve the code
and get rid of the need for these DT properties. Mind you that you still
need to keep the code to parse them, because presumably Exynos relies on
them. But depending on how you split up the driver you might be able to
restrict these compatibility hacks to Exynos and not carry them forward
into your new driver.
Okay, thanks for your remind ;)
+ 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.
I agree that dealing with these in a PHY driver sounds like the better
option. However, I still think that the dp-phy prefix is redundant. The
names are in a per-driver scope, so dp-phy is implied by the device
tree binding and driver already. You could simply use shorter names such
as phy and 24m for example.
Also note that the clock provider will already have the proper names for
these, so the clock tree will end up showing the provider names. The
names in the binding are merely the consumer names.
Agree, thanks
- Yakir
Thierry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Thierry,
在 2015/8/10 18:00, Thierry Reding 写道:
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
[...]
edp: edp@ff97 {
[...]
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
[...]
edp: edp@ff97 {
[...]
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
signature.asc
Description: PGP signature
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Heiko,
在 2015/8/10 20:08, Heiko Stübner 写道:
Hi Yakir,
Am Samstag, 8. August 2015, 11:54:38 schrieb Yakir Yang:
+static int rockchip_dp_init(struct rockchip_dp_device *dp)
+{
+ struct device *dev = dp-dev;
+ struct device_node *np = dev-of_node;
+ int ret;
+
+ dp-grf = syscon_regmap_lookup_by_phandle(np, rockchip,grf);
+ if (IS_ERR(dp-grf)) {
+ dev_err(dev,
+ rk3288-dp needs rockchip,grf property\n);
+ return PTR_ERR(dp-grf);
+ }
+
+ dp-clk_dp = devm_clk_get(dev, clk_dp);
I've looked at the manual, but couldn't find an actual clock-name
used there. Is it really clk_dp or should it just be dp?
This should be clk_dp, not dp.
Cause analogix_dp_core would need a clock name with dp, so I would
rather to pasted my rockchip-dp node here before I add dt-bindings in
next version ;)
The clock we name PCLK_EDP_CTRL in the clock controller is probably the clock
supplying the APB interface and named pclk already in the Figure 3-2
DP_TXclock domain diagram on page 19 of the manual. So your clk_dp should
actually be pclk.
So you would have dp, dp_24m and pclk for the 3 supplying clocks.
Oh, yes, pclk is for APB interface, and sclk_edp for IP controller,
and sclk_edp_24m for DP PHY,
thanks for your explain.
So for now, I would pass sclk_edp to edp in analogix_dp, and
sclk_edp_24m to dp-phy_24m
in phy-rockchip-dp.c, and pclk_edp to pclk in analogix_dp-rockchip.c.
edp: edp@ff97 {
compatible = rockchip,rk3288-dp;
reg = 0xff97 0x4000;
interrupts = GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH;
clocks = cru SCLK_EDP, cru SCLK_EDP_24M, cru
PCLK_EDP_CTRL;
clock-names = clk_dp, clk_dp_24m, dp;
rockchip,grf = grf;
resets = cru 111;
reset-names = dp;
power-domains = power RK3288_PD_VIO;
status = disabled;
hsync-active-high = 0;
vsync-active-high = 0;
interlaced = 0;
samsung,color-space = 0;
samsung,dynamic-range = 0;
samsung,ycbcr-coeff = 0;
samsung,color-depth = 1;
samsung,link-rate = 0x0a;
samsung,lane-count = 1;
Thierry already said, that these should probably be somehow auto-detected.
Properties needing to stay around should probably also be analogix,... with
a fallback to not break Samsung devicetrees, so
look for analogix,foo!, if not found try samsung,foo
Okay, it's better to rename to analogxi..., done.
ports {
edp_in: port {
#address-cells = 1;
#size-cells = 0;
edp_in_vopb: endpoint@0 {
reg = 0;
remote-endpoint = vopb_out_edp;
};
};
};
+
+ 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 ?
As Thierry said, please don't add prefixes.
Okay, so is it okay to rename them to dp, dp-phy-24m, pclk ?
+ if (IS_ERR(dp-clk_24m)) {
+ dev_err(dev, cannot get clk_dp_24m\n);
+ return PTR_ERR(dp-clk_24m);
+ }
I think you're missing the pclk here (PCLK_EDP_CTRL) or is this part of
something else?
Whops, as I refered in commit message I leave pclk_dp to
analogix_dp_core driver ;-)
The reason why I want to leave pclk is I thought this clock is more like
analogix dp
core driver want, like a IP controller clock (whatever analogix_dp do
need a clock
named with dp).
Hmm, I'd think what the core (and Samsung) driver use as dp clock is
probably the generic clock for the IP and not the pclk for the APB interface.
So I think it still should be dp for the core and dp_24m + pclk for the
rockchip part?
Yes, I think you are right, thanks ;)
+
+ dp-rst = devm_reset_control_get(dev, dp);
+ if (IS_ERR(dp-rst)) {
+ dev_err(dev, failed to get reset\n);
+ return PTR_ERR(dp-rst);
+ }
+
+ ret = rockchip_dp_clk_enable(dp);
+ if (ret 0) {
+ dev_err(dp-dev, cannot enable dp clk %d\n, ret);
+ return ret;
+ }
+
+ ret = rockchip_dp_pre_init(dp);
+ if (ret 0) {
+ dev_err(dp-dev, failed to pre init %d\n, ret);
+ return ret;
+ }
+
+ return 0;
+}
[...]
+static int rockchip_dp_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct device_node *panel_node;
+
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
On Mon, Aug 10, 2015 at 08:59:44PM +0800, Yakir Yang wrote:
Hi Thierry,
在 2015/8/10 18:00, Thierry Reding 写道:
On Sat, Aug 08, 2015 at 11:54:38AM +0800, Yakir Yang wrote:
[...]
edp: edp@ff97 {
[...]
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.
The problem with these is that if you keep them in for your initial
submission, you can never (or only under extreme pain) remove them.
Anything in DTB needs to be effectively supported forever.
Also since these don't make sense to hard-code, just improve the code
and get rid of the need for these DT properties. Mind you that you still
need to keep the code to parse them, because presumably Exynos relies on
them. But depending on how you split up the driver you might be able to
restrict these compatibility hacks to Exynos and not carry them forward
into your new driver.
+ 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.
I agree that dealing with these in a PHY driver sounds like the better
option. However, I still think that the dp-phy prefix is redundant. The
names are in a per-driver scope, so dp-phy is implied by the device
tree binding and driver already. You could simply use shorter names such
as phy and 24m for example.
Also note that the clock provider will already have the proper names for
these, so the clock tree will end up showing the provider names. The
names in the binding are merely the consumer names.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Yakir,
Am Samstag, 8. August 2015, 11:54:38 schrieb Yakir Yang:
+static int rockchip_dp_init(struct rockchip_dp_device *dp)
+{
+ struct device *dev = dp-dev;
+ struct device_node *np = dev-of_node;
+ int ret;
+
+ dp-grf = syscon_regmap_lookup_by_phandle(np, rockchip,grf);
+ if (IS_ERR(dp-grf)) {
+ dev_err(dev,
+ rk3288-dp needs rockchip,grf property\n);
+ return PTR_ERR(dp-grf);
+ }
+
+ dp-clk_dp = devm_clk_get(dev, clk_dp);
I've looked at the manual, but couldn't find an actual clock-name
used there. Is it really clk_dp or should it just be dp?
This should be clk_dp, not dp.
Cause analogix_dp_core would need a clock name with dp, so I would
rather to pasted my rockchip-dp node here before I add dt-bindings in
next version ;)
The clock we name PCLK_EDP_CTRL in the clock controller is probably the clock
supplying the APB interface and named pclk already in the Figure 3-2
DP_TXclock domain diagram on page 19 of the manual. So your clk_dp should
actually be pclk.
So you would have dp, dp_24m and pclk for the 3 supplying clocks.
edp: edp@ff97 {
compatible = rockchip,rk3288-dp;
reg = 0xff97 0x4000;
interrupts = GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH;
clocks = cru SCLK_EDP, cru SCLK_EDP_24M, cru
PCLK_EDP_CTRL;
clock-names = clk_dp, clk_dp_24m, dp;
rockchip,grf = grf;
resets = cru 111;
reset-names = dp;
power-domains = power RK3288_PD_VIO;
status = disabled;
hsync-active-high = 0;
vsync-active-high = 0;
interlaced = 0;
samsung,color-space = 0;
samsung,dynamic-range = 0;
samsung,ycbcr-coeff = 0;
samsung,color-depth = 1;
samsung,link-rate = 0x0a;
samsung,lane-count = 1;
Thierry already said, that these should probably be somehow auto-detected.
Properties needing to stay around should probably also be analogix,... with
a fallback to not break Samsung devicetrees, so
look for analogix,foo!, if not found try samsung,foo
ports {
edp_in: port {
#address-cells = 1;
#size-cells = 0;
edp_in_vopb: endpoint@0 {
reg = 0;
remote-endpoint = vopb_out_edp;
};
};
};
+
+ 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 ?
As Thierry said, please don't add prefixes.
+ if (IS_ERR(dp-clk_24m)) {
+ dev_err(dev, cannot get clk_dp_24m\n);
+ return PTR_ERR(dp-clk_24m);
+ }
I think you're missing the pclk here (PCLK_EDP_CTRL) or is this part of
something else?
Whops, as I refered in commit message I leave pclk_dp to
analogix_dp_core driver ;-)
The reason why I want to leave pclk is I thought this clock is more like
analogix dp
core driver want, like a IP controller clock (whatever analogix_dp do
need a clock
named with dp).
Hmm, I'd think what the core (and Samsung) driver use as dp clock is
probably the generic clock for the IP and not the pclk for the APB interface.
So I think it still should be dp for the core and dp_24m + pclk for the
rockchip part?
+
+ dp-rst = devm_reset_control_get(dev, dp);
+ if (IS_ERR(dp-rst)) {
+ dev_err(dev, failed to get reset\n);
+ return PTR_ERR(dp-rst);
+ }
+
+ ret = rockchip_dp_clk_enable(dp);
+ if (ret 0) {
+ dev_err(dp-dev, cannot enable dp clk %d\n, ret);
+ return ret;
+ }
+
+ ret = rockchip_dp_pre_init(dp);
+ if (ret 0) {
+ dev_err(dp-dev, failed to pre init %d\n, ret);
+ return ret;
+ }
+
+ return 0;
+}
[...]
+static int rockchip_dp_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct device_node *panel_node;
+ struct rockchip_dp_device *dp;
+ struct drm_panel *panel;
+
+ panel_node = of_parse_phandle(dev-of_node, rockchip,panel, 0);
+ if (!panel_node) {
+ DRM_ERROR(failed to find rockchip,panel dt node\n);
+ return -ENODEV;
+ }
Personally I would prefer to continue with the of-graph framework to
attach the panel instead of defining a special node. But I'm not
authorative on this. But that way the dts could then look like [0].
I've sucessfully modified the driver currently in use
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Hekio,
在 2015/8/8 6:46, Heiko Stübner 写道:
Hi Yakir,
I think this Rockchip portion is missing a devicetree binding.
Oh, thanks, I would complete it in next ;)
You have the ability to power down the actual edp phy by using
grf_edp_iddq_en from GRF_SOC_CON12. This is similar to how the
rk3288 usb-phy gets put into a deeper state. So maybe you could
provide a phy driver (drivers/phy) for this similar to what the
exynos-dp does.
Okay, so I need to add a new phy-rockchip-dp.c, include
power_on/power_off ops, thanks.
Some more stuff inline. But I guess by no means complete, as I'm
still trying to integrate this into my development-tree.
Am Freitag, 7. August 2015, 05:46:20 schrieb Yakir Yang:
Rockchip have three clocks for dp controller, we leave pclk_edp
to analogix_dp driver control, and keep the sclk_edp_24m and
sclk_edp in platform driver.
Signed-off-by: Yakir Yang
---
Changes in v2: None
drivers/gpu/drm/rockchip/Kconfig| 10 +
drivers/gpu/drm/rockchip/Makefile | 1 +
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 419
3 files changed, 430 insertions(+)
create mode 100644 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
diff --git a/drivers/gpu/drm/rockchip/Kconfig
b/drivers/gpu/drm/rockchip/Kconfig index 35215f6..096ed77 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -25,3 +25,13 @@ config ROCKCHIP_DW_HDMI
for the Synopsys DesignWare HDMI driver. If you want to
enable HDMI on RK3288 based SoC, you should selet this
option.
+
+
nit: double blank line
Done,
+config ROCKCHIP_ANALOGIX_DP
+tristate "Rockchip specific extensions for Analogix DP driver"
+depends on DRM_ROCKCHIP
+select DRM_ANALOGIX_DP
+help
+ This selects support for Rockchip SoC specific extensions
+ for the Analogix Core DP driver. If you want to enable DP
+ on RK3288 based SoC, you should selet this option.
[...]
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c new file mode 100644
index 000..2f86e5e
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -0,0 +1,419 @@
+/*
+ * Rockchip SoC DP (Display Port) interface driver.
+ *
+ * Copyright (C) Fuzhou Rockchip Electronics Co., Ltd.
+ * Author: Andy Yan
+ * Yakir Yang
+ * Jeff Chen
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+
+#include "rockchip_drm_drv.h"
+#include "rockchip_drm_vop.h"
+
+#define encoder_to_dp(c) \
+ container_of(c, struct rockchip_dp_device, encoder)
+
+#define plat_data_to_dp(pd) \
+ container_of(pd, struct rockchip_dp_device, plat_data)
+
+/* dp grf register offset */
+#define DP_VOP_SEL 0x025c /* grf_soc_con6 */
+#define DP_REF_CLK_SEL 0x0274 /* grf_soc_con12 */
+
+#define GRF_DP_REF_CLK_SEL_INTERBIT(4)
+#define DP_SEL_VOP_LIT BIT(5)
+
+struct rockchip_dp_device {
+ struct drm_device*drm_dev;
+ struct device*dev;
+ struct drm_encoder encoder;
+ struct drm_display_mode mode;
+
+ struct clk *clk_dp;
+ struct clk *clk_24m_parent;
this clk_24m_parent does not seem used at all
Done,
+ struct clk *clk_24m;
+ struct regmap*grf;
+ struct reset_control *rst;
+
+ struct analogix_dp_plat_data plat_data;
+};
+
[...]
+static int rockchip_dp_init(struct rockchip_dp_device *dp)
+{
+ struct device *dev = dp->dev;
+ struct device_node *np = dev->of_node;
+ int ret;
+
+ dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
+ if (IS_ERR(dp->grf)) {
+ dev_err(dev,
+ "rk3288-dp needs rockchip,grf property\n");
+ return PTR_ERR(dp->grf);
+ }
+
+ dp->clk_dp = devm_clk_get(dev, "clk_dp");
I've looked at the manual, but couldn't find an actual clock-name
used there. Is it really "clk_dp" or should it just be "dp"?
This should be "clk_dp", not "dp".
Cause analogix_dp_core would need a clock name with "dp", so I would
rather to pasted my rockchip-dp node here before I add dt-bindings in
next version ;)
edp: edp@ff97 {
compatible = "rockchip,rk3288-dp";
reg = <0xff97 0x4000>;
interrupts = ;
clocks = < SCLK_EDP>, < SCLK_EDP_24M>, <
PCLK_EDP_CTRL>;
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Yakir,
I think this Rockchip portion is missing a devicetree binding.
You have the ability to power down the actual edp phy by using
grf_edp_iddq_en from GRF_SOC_CON12. This is similar to how the
rk3288 usb-phy gets put into a deeper state. So maybe you could
provide a phy driver (drivers/phy) for this similar to what the
exynos-dp does.
Some more stuff inline. But I guess by no means complete, as I'm
still trying to integrate this into my development-tree.
Am Freitag, 7. August 2015, 05:46:20 schrieb Yakir Yang:
> Rockchip have three clocks for dp controller, we leave pclk_edp
> to analogix_dp driver control, and keep the sclk_edp_24m and
> sclk_edp in platform driver.
>
> Signed-off-by: Yakir Yang
> ---
> Changes in v2: None
>
> drivers/gpu/drm/rockchip/Kconfig| 10 +
> drivers/gpu/drm/rockchip/Makefile | 1 +
> drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 419
> 3 files changed, 430 insertions(+)
> create mode 100644 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>
> diff --git a/drivers/gpu/drm/rockchip/Kconfig
> b/drivers/gpu/drm/rockchip/Kconfig index 35215f6..096ed77 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -25,3 +25,13 @@ config ROCKCHIP_DW_HDMI
> for the Synopsys DesignWare HDMI driver. If you want to
> enable HDMI on RK3288 based SoC, you should selet this
> option.
> +
> +
nit: double blank line
> +config ROCKCHIP_ANALOGIX_DP
> +tristate "Rockchip specific extensions for Analogix DP driver"
> +depends on DRM_ROCKCHIP
> +select DRM_ANALOGIX_DP
> +help
> + This selects support for Rockchip SoC specific extensions
> + for the Analogix Core DP driver. If you want to enable DP
> + on RK3288 based SoC, you should selet this option.
[...]
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c new file mode 100644
> index 000..2f86e5e
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -0,0 +1,419 @@
> +/*
> + * Rockchip SoC DP (Display Port) interface driver.
> + *
> + * Copyright (C) Fuzhou Rockchip Electronics Co., Ltd.
> + * Author: Andy Yan
> + * Yakir Yang
> + * Jeff Chen
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +
> +#include "rockchip_drm_drv.h"
> +#include "rockchip_drm_vop.h"
> +
> +#define encoder_to_dp(c) \
> + container_of(c, struct rockchip_dp_device, encoder)
> +
> +#define plat_data_to_dp(pd) \
> + container_of(pd, struct rockchip_dp_device, plat_data)
> +
> +/* dp grf register offset */
> +#define DP_VOP_SEL 0x025c /* grf_soc_con6 */
> +#define DP_REF_CLK_SEL 0x0274 /* grf_soc_con12 */
> +
> +#define GRF_DP_REF_CLK_SEL_INTERBIT(4)
> +#define DP_SEL_VOP_LIT BIT(5)
> +
> +struct rockchip_dp_device {
> + struct drm_device*drm_dev;
> + struct device*dev;
> + struct drm_encoder encoder;
> + struct drm_display_mode mode;
> +
> + struct clk *clk_dp;
> + struct clk *clk_24m_parent;
this clk_24m_parent does not seem used at all
> + struct clk *clk_24m;
> + struct regmap*grf;
> + struct reset_control *rst;
> +
> + struct analogix_dp_plat_data plat_data;
> +};
> +
[...]
> +static int rockchip_dp_init(struct rockchip_dp_device *dp)
> +{
> + struct device *dev = dp->dev;
> + struct device_node *np = dev->of_node;
> + int ret;
> +
> + dp->grf = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> + if (IS_ERR(dp->grf)) {
> + dev_err(dev,
> + "rk3288-dp needs rockchip,grf property\n");
> + return PTR_ERR(dp->grf);
> + }
> +
> + dp->clk_dp = devm_clk_get(dev, "clk_dp");
I've looked at the manual, but couldn't find an actual clock-name
used there. Is it really "clk_dp" or should it just be "dp"?
> + if (IS_ERR(dp->clk_dp)) {
> + dev_err(dev, "cannot get clk_dp\n");
> + return PTR_ERR(dp->clk_dp);
> + }
> +
> + dp->clk_24m = devm_clk_get(dev, "clk_dp_24m");
Same here, maybe "dp_24m".
> + if (IS_ERR(dp->clk_24m)) {
> + dev_err(dev, "cannot get clk_dp_24m\n");
> + return PTR_ERR(dp->clk_24m);
> + }
I think you're missing the pclk here (PCLK_EDP_CTRL) or is this part of
something else?
>
[PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Rockchip have three clocks for dp controller, we leave pclk_edp
to analogix_dp driver control, and keep the sclk_edp_24m and
sclk_edp in platform driver.
Signed-off-by: Yakir Yang
---
Changes in v2: None
drivers/gpu/drm/rockchip/Kconfig| 10 +
drivers/gpu/drm/rockchip/Makefile | 1 +
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 419
3 files changed, 430 insertions(+)
create mode 100644 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 35215f6..096ed77 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -25,3 +25,13 @@ config ROCKCHIP_DW_HDMI
for the Synopsys DesignWare HDMI driver. If you want to
enable HDMI on RK3288 based SoC, you should selet this
option.
+
+
+config ROCKCHIP_ANALOGIX_DP
+tristate "Rockchip specific extensions for Analogix DP driver"
+depends on DRM_ROCKCHIP
+select DRM_ANALOGIX_DP
+help
+ This selects support for Rockchip SoC specific extensions
+ for the Analogix Core DP driver. If you want to enable DP
+ on RK3288 based SoC, you should selet this option.
diff --git a/drivers/gpu/drm/rockchip/Makefile
b/drivers/gpu/drm/rockchip/Makefile
index f3d8a19..8ad01fb 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -6,5 +6,6 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o
rockchip_drm_fbdev.o \
rockchip_drm_gem.o
obj-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
+obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_drm_vop.o
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
new file mode 100644
index 000..2f86e5e
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -0,0 +1,419 @@
+/*
+ * Rockchip SoC DP (Display Port) interface driver.
+ *
+ * Copyright (C) Fuzhou Rockchip Electronics Co., Ltd.
+ * Author: Andy Yan
+ * Yakir Yang
+ * Jeff Chen
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+
+#include "rockchip_drm_drv.h"
+#include "rockchip_drm_vop.h"
+
+#define encoder_to_dp(c) \
+ container_of(c, struct rockchip_dp_device, encoder)
+
+#define plat_data_to_dp(pd) \
+ container_of(pd, struct rockchip_dp_device, plat_data)
+
+/* dp grf register offset */
+#define DP_VOP_SEL 0x025c /* grf_soc_con6 */
+#define DP_REF_CLK_SEL 0x0274 /* grf_soc_con12 */
+
+#define GRF_DP_REF_CLK_SEL_INTERBIT(4)
+#define DP_SEL_VOP_LIT BIT(5)
+
+struct rockchip_dp_device {
+ struct drm_device*drm_dev;
+ struct device*dev;
+ struct drm_encoder encoder;
+ struct drm_display_mode mode;
+
+ struct clk *clk_dp;
+ struct clk *clk_24m_parent;
+ struct clk *clk_24m;
+ struct regmap*grf;
+ struct reset_control *rst;
+
+ struct analogix_dp_plat_data plat_data;
+};
+
+static int rockchip_dp_clk_enable(struct rockchip_dp_device *dp)
+{
+ int ret = 0;
+
+ ret = clk_prepare_enable(dp->clk_dp);
+ if (ret < 0) {
+ dev_err(dp->dev, "cannot enable clk_dp %d\n", ret);
+ goto err_clk_dp;
+ }
+
+ ret = clk_set_rate(dp->clk_24m, 2400);
+ if (ret < 0) {
+ dev_err(dp->dev, "cannot set dp clk_24m %d\n",
+ ret);
+ goto err_clk_24m;
+ }
+
+ ret = clk_prepare_enable(dp->clk_24m);
+ if (ret < 0) {
+ dev_err(dp->dev, "cannot enable dp clk_24m %d\n",
+ ret);
+ goto err_clk_24m;
+ }
+
+ return 0;
+
+err_clk_24m:
+ clk_disable_unprepare(dp->clk_dp);
+err_clk_dp:
+ return ret;
+}
+
+static int rockchip_dp_clk_disable(struct rockchip_dp_device *dp)
+{
+ clk_disable_unprepare(dp->clk_dp);
+ clk_disable_unprepare(dp->clk_24m);
+
+ return 0;
+}
+
+static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
+{
+ u32 val;
+ int ret;
+
+ val = GRF_DP_REF_CLK_SEL_INTER | (GRF_DP_REF_CLK_SEL_INTER << 16);
+ ret = regmap_write(dp->grf, DP_REF_CLK_SEL, val);
+ if (ret != 0) {
+ dev_err(dp->dev, "Could not write to GRF: %d\n", ret);
+ return ret;
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Yakir,
I think this Rockchip portion is missing a devicetree binding.
You have the ability to power down the actual edp phy by using
grf_edp_iddq_en from GRF_SOC_CON12. This is similar to how the
rk3288 usb-phy gets put into a deeper state. So maybe you could
provide a phy driver (drivers/phy) for this similar to what the
exynos-dp does.
Some more stuff inline. But I guess by no means complete, as I'm
still trying to integrate this into my development-tree.
Am Freitag, 7. August 2015, 05:46:20 schrieb Yakir Yang:
Rockchip have three clocks for dp controller, we leave pclk_edp
to analogix_dp driver control, and keep the sclk_edp_24m and
sclk_edp in platform driver.
Signed-off-by: Yakir Yang y...@rock-chips.com
---
Changes in v2: None
drivers/gpu/drm/rockchip/Kconfig| 10 +
drivers/gpu/drm/rockchip/Makefile | 1 +
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 419
3 files changed, 430 insertions(+)
create mode 100644 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
diff --git a/drivers/gpu/drm/rockchip/Kconfig
b/drivers/gpu/drm/rockchip/Kconfig index 35215f6..096ed77 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -25,3 +25,13 @@ config ROCKCHIP_DW_HDMI
for the Synopsys DesignWare HDMI driver. If you want to
enable HDMI on RK3288 based SoC, you should selet this
option.
+
+
nit: double blank line
+config ROCKCHIP_ANALOGIX_DP
+tristate Rockchip specific extensions for Analogix DP driver
+depends on DRM_ROCKCHIP
+select DRM_ANALOGIX_DP
+help
+ This selects support for Rockchip SoC specific extensions
+ for the Analogix Core DP driver. If you want to enable DP
+ on RK3288 based SoC, you should selet this option.
[...]
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c new file mode 100644
index 000..2f86e5e
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -0,0 +1,419 @@
+/*
+ * Rockchip SoC DP (Display Port) interface driver.
+ *
+ * Copyright (C) Fuzhou Rockchip Electronics Co., Ltd.
+ * Author: Andy Yan andy@rock-chips.com
+ * Yakir Yang y...@rock-chips.com
+ * Jeff Chen jeff.c...@rock-chips.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include drm/drmP.h
+#include drm/drm_crtc_helper.h
+#include drm/drm_panel.h
+#include drm/drm_of.h
+#include drm/drm_dp_helper.h
+
+#include linux/component.h
+#include linux/clk.h
+#include linux/mfd/syscon.h
+#include linux/regmap.h
+#include linux/reset.h
+
+#include video/of_videomode.h
+#include video/videomode.h
+
+#include drm/bridge/analogix_dp.h
+
+#include rockchip_drm_drv.h
+#include rockchip_drm_vop.h
+
+#define encoder_to_dp(c) \
+ container_of(c, struct rockchip_dp_device, encoder)
+
+#define plat_data_to_dp(pd) \
+ container_of(pd, struct rockchip_dp_device, plat_data)
+
+/* dp grf register offset */
+#define DP_VOP_SEL 0x025c /* grf_soc_con6 */
+#define DP_REF_CLK_SEL 0x0274 /* grf_soc_con12 */
+
+#define GRF_DP_REF_CLK_SEL_INTERBIT(4)
+#define DP_SEL_VOP_LIT BIT(5)
+
+struct rockchip_dp_device {
+ struct drm_device*drm_dev;
+ struct device*dev;
+ struct drm_encoder encoder;
+ struct drm_display_mode mode;
+
+ struct clk *clk_dp;
+ struct clk *clk_24m_parent;
this clk_24m_parent does not seem used at all
+ struct clk *clk_24m;
+ struct regmap*grf;
+ struct reset_control *rst;
+
+ struct analogix_dp_plat_data plat_data;
+};
+
[...]
+static int rockchip_dp_init(struct rockchip_dp_device *dp)
+{
+ struct device *dev = dp-dev;
+ struct device_node *np = dev-of_node;
+ int ret;
+
+ dp-grf = syscon_regmap_lookup_by_phandle(np, rockchip,grf);
+ if (IS_ERR(dp-grf)) {
+ dev_err(dev,
+ rk3288-dp needs rockchip,grf property\n);
+ return PTR_ERR(dp-grf);
+ }
+
+ dp-clk_dp = devm_clk_get(dev, clk_dp);
I've looked at the manual, but couldn't find an actual clock-name
used there. Is it really clk_dp or should it just be dp?
+ if (IS_ERR(dp-clk_dp)) {
+ dev_err(dev, cannot get clk_dp\n);
+ return PTR_ERR(dp-clk_dp);
+ }
+
+ dp-clk_24m = devm_clk_get(dev, clk_dp_24m);
Same here, maybe dp_24m.
+ if (IS_ERR(dp-clk_24m)) {
+ dev_err(dev, cannot get clk_dp_24m\n);
+
Re: [PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Hi Hekio,
在 2015/8/8 6:46, Heiko Stübner 写道:
Hi Yakir,
I think this Rockchip portion is missing a devicetree binding.
Oh, thanks, I would complete it in next ;)
You have the ability to power down the actual edp phy by using
grf_edp_iddq_en from GRF_SOC_CON12. This is similar to how the
rk3288 usb-phy gets put into a deeper state. So maybe you could
provide a phy driver (drivers/phy) for this similar to what the
exynos-dp does.
Okay, so I need to add a new phy-rockchip-dp.c, include
power_on/power_off ops, thanks.
Some more stuff inline. But I guess by no means complete, as I'm
still trying to integrate this into my development-tree.
Am Freitag, 7. August 2015, 05:46:20 schrieb Yakir Yang:
Rockchip have three clocks for dp controller, we leave pclk_edp
to analogix_dp driver control, and keep the sclk_edp_24m and
sclk_edp in platform driver.
Signed-off-by: Yakir Yang y...@rock-chips.com
---
Changes in v2: None
drivers/gpu/drm/rockchip/Kconfig| 10 +
drivers/gpu/drm/rockchip/Makefile | 1 +
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 419
3 files changed, 430 insertions(+)
create mode 100644 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
diff --git a/drivers/gpu/drm/rockchip/Kconfig
b/drivers/gpu/drm/rockchip/Kconfig index 35215f6..096ed77 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -25,3 +25,13 @@ config ROCKCHIP_DW_HDMI
for the Synopsys DesignWare HDMI driver. If you want to
enable HDMI on RK3288 based SoC, you should selet this
option.
+
+
nit: double blank line
Done,
+config ROCKCHIP_ANALOGIX_DP
+tristate Rockchip specific extensions for Analogix DP driver
+depends on DRM_ROCKCHIP
+select DRM_ANALOGIX_DP
+help
+ This selects support for Rockchip SoC specific extensions
+ for the Analogix Core DP driver. If you want to enable DP
+ on RK3288 based SoC, you should selet this option.
[...]
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c new file mode 100644
index 000..2f86e5e
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -0,0 +1,419 @@
+/*
+ * Rockchip SoC DP (Display Port) interface driver.
+ *
+ * Copyright (C) Fuzhou Rockchip Electronics Co., Ltd.
+ * Author: Andy Yan andy@rock-chips.com
+ * Yakir Yang y...@rock-chips.com
+ * Jeff Chen jeff.c...@rock-chips.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include drm/drmP.h
+#include drm/drm_crtc_helper.h
+#include drm/drm_panel.h
+#include drm/drm_of.h
+#include drm/drm_dp_helper.h
+
+#include linux/component.h
+#include linux/clk.h
+#include linux/mfd/syscon.h
+#include linux/regmap.h
+#include linux/reset.h
+
+#include video/of_videomode.h
+#include video/videomode.h
+
+#include drm/bridge/analogix_dp.h
+
+#include rockchip_drm_drv.h
+#include rockchip_drm_vop.h
+
+#define encoder_to_dp(c) \
+ container_of(c, struct rockchip_dp_device, encoder)
+
+#define plat_data_to_dp(pd) \
+ container_of(pd, struct rockchip_dp_device, plat_data)
+
+/* dp grf register offset */
+#define DP_VOP_SEL 0x025c /* grf_soc_con6 */
+#define DP_REF_CLK_SEL 0x0274 /* grf_soc_con12 */
+
+#define GRF_DP_REF_CLK_SEL_INTERBIT(4)
+#define DP_SEL_VOP_LIT BIT(5)
+
+struct rockchip_dp_device {
+ struct drm_device*drm_dev;
+ struct device*dev;
+ struct drm_encoder encoder;
+ struct drm_display_mode mode;
+
+ struct clk *clk_dp;
+ struct clk *clk_24m_parent;
this clk_24m_parent does not seem used at all
Done,
+ struct clk *clk_24m;
+ struct regmap*grf;
+ struct reset_control *rst;
+
+ struct analogix_dp_plat_data plat_data;
+};
+
[...]
+static int rockchip_dp_init(struct rockchip_dp_device *dp)
+{
+ struct device *dev = dp-dev;
+ struct device_node *np = dev-of_node;
+ int ret;
+
+ dp-grf = syscon_regmap_lookup_by_phandle(np, rockchip,grf);
+ if (IS_ERR(dp-grf)) {
+ dev_err(dev,
+ rk3288-dp needs rockchip,grf property\n);
+ return PTR_ERR(dp-grf);
+ }
+
+ dp-clk_dp = devm_clk_get(dev, clk_dp);
I've looked at the manual, but couldn't find an actual clock-name
used there. Is it really clk_dp or should it just be dp?
This should be clk_dp, not dp.
Cause analogix_dp_core would need a clock name with dp, so I would
rather to pasted my rockchip-dp node here before I add
[PATCH v2 4/8] drm: rockchip/dp: add rockchip platform dp driver
Rockchip have three clocks for dp controller, we leave pclk_edp
to analogix_dp driver control, and keep the sclk_edp_24m and
sclk_edp in platform driver.
Signed-off-by: Yakir Yang y...@rock-chips.com
---
Changes in v2: None
drivers/gpu/drm/rockchip/Kconfig| 10 +
drivers/gpu/drm/rockchip/Makefile | 1 +
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 419
3 files changed, 430 insertions(+)
create mode 100644 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 35215f6..096ed77 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -25,3 +25,13 @@ config ROCKCHIP_DW_HDMI
for the Synopsys DesignWare HDMI driver. If you want to
enable HDMI on RK3288 based SoC, you should selet this
option.
+
+
+config ROCKCHIP_ANALOGIX_DP
+tristate Rockchip specific extensions for Analogix DP driver
+depends on DRM_ROCKCHIP
+select DRM_ANALOGIX_DP
+help
+ This selects support for Rockchip SoC specific extensions
+ for the Analogix Core DP driver. If you want to enable DP
+ on RK3288 based SoC, you should selet this option.
diff --git a/drivers/gpu/drm/rockchip/Makefile
b/drivers/gpu/drm/rockchip/Makefile
index f3d8a19..8ad01fb 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -6,5 +6,6 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o
rockchip_drm_fbdev.o \
rockchip_drm_gem.o
obj-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
+obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_drm_vop.o
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
new file mode 100644
index 000..2f86e5e
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -0,0 +1,419 @@
+/*
+ * Rockchip SoC DP (Display Port) interface driver.
+ *
+ * Copyright (C) Fuzhou Rockchip Electronics Co., Ltd.
+ * Author: Andy Yan andy@rock-chips.com
+ * Yakir Yang y...@rock-chips.com
+ * Jeff Chen jeff.c...@rock-chips.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include drm/drmP.h
+#include drm/drm_crtc_helper.h
+#include drm/drm_panel.h
+#include drm/drm_of.h
+#include drm/drm_dp_helper.h
+
+#include linux/component.h
+#include linux/clk.h
+#include linux/mfd/syscon.h
+#include linux/regmap.h
+#include linux/reset.h
+
+#include video/of_videomode.h
+#include video/videomode.h
+
+#include drm/bridge/analogix_dp.h
+
+#include rockchip_drm_drv.h
+#include rockchip_drm_vop.h
+
+#define encoder_to_dp(c) \
+ container_of(c, struct rockchip_dp_device, encoder)
+
+#define plat_data_to_dp(pd) \
+ container_of(pd, struct rockchip_dp_device, plat_data)
+
+/* dp grf register offset */
+#define DP_VOP_SEL 0x025c /* grf_soc_con6 */
+#define DP_REF_CLK_SEL 0x0274 /* grf_soc_con12 */
+
+#define GRF_DP_REF_CLK_SEL_INTERBIT(4)
+#define DP_SEL_VOP_LIT BIT(5)
+
+struct rockchip_dp_device {
+ struct drm_device*drm_dev;
+ struct device*dev;
+ struct drm_encoder encoder;
+ struct drm_display_mode mode;
+
+ struct clk *clk_dp;
+ struct clk *clk_24m_parent;
+ struct clk *clk_24m;
+ struct regmap*grf;
+ struct reset_control *rst;
+
+ struct analogix_dp_plat_data plat_data;
+};
+
+static int rockchip_dp_clk_enable(struct rockchip_dp_device *dp)
+{
+ int ret = 0;
+
+ ret = clk_prepare_enable(dp-clk_dp);
+ if (ret 0) {
+ dev_err(dp-dev, cannot enable clk_dp %d\n, ret);
+ goto err_clk_dp;
+ }
+
+ ret = clk_set_rate(dp-clk_24m, 2400);
+ if (ret 0) {
+ dev_err(dp-dev, cannot set dp clk_24m %d\n,
+ ret);
+ goto err_clk_24m;
+ }
+
+ ret = clk_prepare_enable(dp-clk_24m);
+ if (ret 0) {
+ dev_err(dp-dev, cannot enable dp clk_24m %d\n,
+ ret);
+ goto err_clk_24m;
+ }
+
+ return 0;
+
+err_clk_24m:
+ clk_disable_unprepare(dp-clk_dp);
+err_clk_dp:
+ return ret;
+}
+
+static int rockchip_dp_clk_disable(struct rockchip_dp_device *dp)
+{
+ clk_disable_unprepare(dp-clk_dp);
+ clk_disable_unprepare(dp-clk_24m);
+
+ return 0;
+}
+
+static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
+{
+ u32 val;
+
