Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On 2022-03-09 08:37, elaine.zhang wrote: hi, 在 2022/3/9 下午4:18, Sascha Hauer 写道: Hi Elaine, On Wed, Mar 09, 2022 at 09:41:39AM +0800, zhangq...@rock-chips.com wrote: hi，all： Let me explain the clock dependency： From the clock tree, pclk_vo0 and hclk_vo0 are completely independent clocks with different parent clocks and different clock frequencies。 But the niu path is : pclk_vo is dependent on hclk_vo, and the pclk_vo niu goes through hclk_vo niu. Thanks, this is the information we are looking for. What is "NIU" btw? I think this is even documented in the Reference Manual. With the right pointer I just found: The NIU (native interface unit) You can see 2.8.6(NIU Clock gating reliance) in TRM. A part of niu clocks have a dependence on another niu clock in order to sharing the internal bus. When these clocks are in use, another niu clock must be opened, and cannot be gated. These clocks and the special clock on which they are relied are as following: Clocks which have dependency The clock which can not be gated - ... pclk_vo_niu, hclk_vo_s_niu hclk_vo_niu ... Yeah, the dependency is this. It may be not very detailed, I don't have permission to publish detailed NIU designs. The clock tree and NIU bus paths are designed independently So there are three solutions to this problem: 1. DTS adds a reference to Hclk while referencing Pclk. 2, The dependent clock is always on, such as HCLK_VO0, but this is not friendly for the system power. 3. Create a non-clock-tree reference. Clk-link, for example, we have an implementation in our internal branch, but Upstream is not sure how to push it. I thought about something similar. That would help us here and on i.MX we have a similar situation: We have one bit that switches multiple clocks. That as well cannot be designed properly in the clock framework currently, but could be modelled with a concept of linked clocks. Doing this sounds like quite a bit of work and discussion though, I don't really like having this as a dependency to mainline the VOP2 driver. I vote for 1. in that case, we could still ignore the hclk in dts later when we have linked clocks. OK so just to clarify, the issue is not just that the upstream clock driver doesn't currently model the NIU clocks, but that even if it did, it would still need to implement some new multi-parent clock type to manage the internal dependency? That's fair enough - I do think that improving the clock driver would be the best long-term goal, but for the scope of this series, having an interim workaround does seem more reasonable now that we understand *why* we need it. Thanks, Robin.
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
hi, 在 2022/3/9 下午4:18, Sascha Hauer 写道: Hi Elaine, On Wed, Mar 09, 2022 at 09:41:39AM +0800, zhangq...@rock-chips.com wrote: hi，all： Let me explain the clock dependency： From the clock tree, pclk_vo0 and hclk_vo0 are completely independent clocks with different parent clocks and different clock frequencies。 But the niu path is : pclk_vo is dependent on hclk_vo, and the pclk_vo niu goes through hclk_vo niu. Thanks, this is the information we are looking for. What is "NIU" btw? I think this is even documented in the Reference Manual. With the right pointer I just found: The NIU (native interface unit) You can see 2.8.6(NIU Clock gating reliance) in TRM. A part of niu clocks have a dependence on another niu clock in order to sharing the internal bus. When these clocks are in use, another niu clock must be opened, and cannot be gated. These clocks and the special clock on which they are relied are as following: Clocks which have dependency The clock which can not be gated - ... pclk_vo_niu, hclk_vo_s_niu hclk_vo_niu ... Yeah, the dependency is this. It may be not very detailed, I don't have permission to publish detailed NIU designs. The clock tree and NIU bus paths are designed independently So there are three solutions to this problem: 1. DTS adds a reference to Hclk while referencing Pclk. 2, The dependent clock is always on, such as HCLK_VO0, but this is not friendly for the system power. 3. Create a non-clock-tree reference. Clk-link, for example, we have an implementation in our internal branch, but Upstream is not sure how to push it. I thought about something similar. That would help us here and on i.MX we have a similar situation: We have one bit that switches multiple clocks. That as well cannot be designed properly in the clock framework currently, but could be modelled with a concept of linked clocks. Doing this sounds like quite a bit of work and discussion though, I don't really like having this as a dependency to mainline the VOP2 driver. I vote for 1. in that case, we could still ignore the hclk in dts later when we have linked clocks. Sascha
Re: Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
Hi Elaine, On Wed, Mar 09, 2022 at 09:41:39AM +0800, zhangq...@rock-chips.com wrote: >hi，all： >Let me explain the clock dependency： >From the clock tree, pclk_vo0 and hclk_vo0 are completely independent >clocks with different parent clocks and different clock frequencies。 >But the niu path is : >pclk_vo is dependent on hclk_vo, and the pclk_vo niu goes through hclk_vo >niu. Thanks, this is the information we are looking for. What is "NIU" btw? I think this is even documented in the Reference Manual. With the right pointer I just found: > A part of niu clocks have a dependence on another niu clock in order to > sharing the internal bus. When these clocks are in use, another niu > clock must be opened, and cannot be gated. These clocks and the special > clock on which they are relied are as following: > > Clocks which have dependency The clock which can not be gated > - > ... > pclk_vo_niu, hclk_vo_s_niu hclk_vo_niu > ... >The clock tree and NIU bus paths are designed independently >So there are three solutions to this problem: >1. DTS adds a reference to Hclk while referencing Pclk. >2, The dependent clock is always on, such as HCLK_VO0, but this is not >friendly for the system power. >3. Create a non-clock-tree reference. Clk-link, for example, we have an >implementation in our internal branch, but Upstream is not sure how to >push it. I thought about something similar. That would help us here and on i.MX we have a similar situation: We have one bit that switches multiple clocks. That as well cannot be designed properly in the clock framework currently, but could be modelled with a concept of linked clocks. Doing this sounds like quite a bit of work and discussion though, I don't really like having this as a dependency to mainline the VOP2 driver. I vote for 1. in that case, we could still ignore the hclk in dts later when we have linked clocks. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
Hi :
On 3/5/22 07:55, Dmitry Osipenko wrote:
On 3/4/22 17:22, Sascha Hauer wrote:
On Wed, Mar 02, 2022 at 12:25:28PM +0100, Sascha Hauer wrote:
On Tue, Mar 01, 2022 at 01:39:31PM +, Robin Murphy wrote:
On 2022-02-28 14:19, Sascha Hauer wrote:
On Fri, Feb 25, 2022 at 02:11:54PM +0100, Sascha Hauer wrote:
On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
On 2022-02-25 11:10, Dmitry Osipenko wrote:
25.02.2022 13:49, Sascha Hauer пишет:
On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
25.02.2022 10:51, Sascha Hauer пишет:
The rk3568 HDMI has an additional clock that needs to be enabled for the
HDMI controller to work. The purpose of that clock is not clear. It is
named "hclk" in the downstream driver, so use the same name.
Signed-off-by: Sascha Hauer
---
Notes:
Changes since v5:
- Use devm_clk_get_optional rather than devm_clk_get
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index fe4f9556239ac..c6c00e8779ab5 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -76,6 +76,7 @@ struct rockchip_hdmi {
const struct rockchip_hdmi_chip_data *chip_data;
struct clk *ref_clk;
struct clk *grf_clk;
+ struct clk *hclk_clk;
struct dw_hdmi *hdmi;
struct regulator *avdd_0v9;
struct regulator *avdd_1v8;
@@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
*hdmi)
return PTR_ERR(hdmi->grf_clk);
}
+ hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
+ if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
Have you tried to investigate the hclk? I'm still thinking that's not
only HDMI that needs this clock and then the hardware description
doesn't look correct.
I am still not sure what you mean. Yes, it's not only the HDMI that
needs this clock. The VOP2 needs it as well and the driver handles that.
I'm curious whether DSI/DP also need that clock to be enabled. If they
do, then you aren't modeling h/w properly AFAICS.
Assuming nobody at Rockchip decided to make things needlessly inconsistent
with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
interface. Usually, if that affected anything other than accessing VOP
registers, indeed it would smell of something being wrong in the clock tree,
but in this case I'd also be suspicious of whether it might have ended up
clocking related GRF registers as well (either directly, or indirectly via
some gate that the clock driver hasn't modelled yet).
Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
hanging when HCLK_VOP is disabled by disabling that clock via sysfs
using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
of that units can't be accessed. However, when I disable HCLK_VOP by
directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
accessing VOP registers hangs, the other units stay functional.
So it seems it must be the parent clock which must be enabled. The
parent clock is hclk_vo. This clock should be handled as part of the
RK3568_PD_VO power domain:
power-domain@RK3568_PD_VO {
reg = ;
clocks = <&cru HCLK_VO>,
<&cru PCLK_VO>,
<&cru ACLK_VOP_PRE>;
pm_qos = <&qos_hdcp>,
<&qos_vop_m0>,
<&qos_vop_m1>;
#power-domain-cells = <0>;
};
Forget this. The clocks in this node are only enabled during enabling or
disabling the power domain, they are disabled again immediately afterwards.
OK, I need HCLK_VO to access the HDMI registers. I verified that by
disabling HCLK_VO at register level (CRU_GATE_CON(20) BIT(1)). The
HDMI registers become inaccessible then. This means I'll replace
HCLK_VOP in the HDMI node with HCLK_VO. Does this sound sane?
Well, it's still a mystery hack overall, and in some ways it seems even more
suspect to be claiming a whole branch of the clock tree rather than a leaf
gate with a specific purpose. I'm really starting to think that the
underlying issue here is a bug in the clock driver, or a hardware mishap
that should logically be worked around by the clock driver, rather than
individual the consumers.
Does it work if you hack the clock driver to think that PCLK_VO is a child
of HCLK_VO? Even if that's not technically true, it would seem to
effectively match the observed behaviour (i.e. all 3 things whose register
access apparently *should* be enabled by a gate off PCLK_VO, seem to also
require HCLK_VO).
Yes, that works as expected. I am not sure though if we really want to
go that path. The pclk rates will become completely bogus with this and
should we have to play with the rates in the future w
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On 3/4/22 17:22, Sascha Hauer wrote:
> On Wed, Mar 02, 2022 at 12:25:28PM +0100, Sascha Hauer wrote:
>> On Tue, Mar 01, 2022 at 01:39:31PM +, Robin Murphy wrote:
>>> On 2022-02-28 14:19, Sascha Hauer wrote:
On Fri, Feb 25, 2022 at 02:11:54PM +0100, Sascha Hauer wrote:
> On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
>> On 2022-02-25 11:10, Dmitry Osipenko wrote:
>>> 25.02.2022 13:49, Sascha Hauer пишет:
On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
> 25.02.2022 10:51, Sascha Hauer пишет:
>> The rk3568 HDMI has an additional clock that needs to be enabled for
>> the
>> HDMI controller to work. The purpose of that clock is not clear. It
>> is
>> named "hclk" in the downstream driver, so use the same name.
>>
>> Signed-off-by: Sascha Hauer
>> ---
>>
>> Notes:
>> Changes since v5:
>> - Use devm_clk_get_optional rather than devm_clk_get
>>
>>drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
>>1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>> index fe4f9556239ac..c6c00e8779ab5 100644
>> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>> @@ -76,6 +76,7 @@ struct rockchip_hdmi {
>> const struct rockchip_hdmi_chip_data *chip_data;
>> struct clk *ref_clk;
>> struct clk *grf_clk;
>> +struct clk *hclk_clk;
>> struct dw_hdmi *hdmi;
>> struct regulator *avdd_0v9;
>> struct regulator *avdd_1v8;
>> @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct
>> rockchip_hdmi *hdmi)
>> return PTR_ERR(hdmi->grf_clk);
>> }
>> +hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
>> +if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
>
> Have you tried to investigate the hclk? I'm still thinking that's not
> only HDMI that needs this clock and then the hardware description
> doesn't look correct.
I am still not sure what you mean. Yes, it's not only the HDMI that
needs this clock. The VOP2 needs it as well and the driver handles
that.
>>>
>>> I'm curious whether DSI/DP also need that clock to be enabled. If they
>>> do, then you aren't modeling h/w properly AFAICS.
>>
>> Assuming nobody at Rockchip decided to make things needlessly
>> inconsistent
>> with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
>> interface. Usually, if that affected anything other than accessing VOP
>> registers, indeed it would smell of something being wrong in the clock
>> tree,
>> but in this case I'd also be suspicious of whether it might have ended up
>> clocking related GRF registers as well (either directly, or indirectly
>> via
>> some gate that the clock driver hasn't modelled yet).
>
> Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
> hanging when HCLK_VOP is disabled by disabling that clock via sysfs
> using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
> of that units can't be accessed. However, when I disable HCLK_VOP by
> directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
> accessing VOP registers hangs, the other units stay functional.
> So it seems it must be the parent clock which must be enabled. The
> parent clock is hclk_vo. This clock should be handled as part of the
> RK3568_PD_VO power domain:
>
> power-domain@RK3568_PD_VO {
> reg = ;
> clocks = <&cru HCLK_VO>,
> <&cru PCLK_VO>,
> <&cru ACLK_VOP_PRE>;
> pm_qos = <&qos_hdcp>,
><&qos_vop_m0>,
><&qos_vop_m1>;
> #power-domain-cells = <0>;
> };
Forget this. The clocks in this node are only enabled during enabling or
disabling the power domain, they are disabled again immediately afterwards.
OK, I need HCLK_VO to access the HDMI registers. I verified that by
disabling HCLK_VO at register level (CRU_GATE_CON(20) BIT(1)). The
HDMI registers become inaccessible then. This means I'll replace
HCLK_VOP in the HDMI node with HCLK_VO. Does this sound sane?
>>>
>>> Well, it's still a mystery hack overall, and in some ways it seems even more
>>> suspect to be claiming a whole branch of the clock tree rather than a leaf
>>> gate with a specific purpose. I'm really starting to think that
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On Wed, Mar 02, 2022 at 12:25:28PM +0100, Sascha Hauer wrote:
> On Tue, Mar 01, 2022 at 01:39:31PM +, Robin Murphy wrote:
> > On 2022-02-28 14:19, Sascha Hauer wrote:
> > > On Fri, Feb 25, 2022 at 02:11:54PM +0100, Sascha Hauer wrote:
> > > > On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
> > > > > On 2022-02-25 11:10, Dmitry Osipenko wrote:
> > > > > > 25.02.2022 13:49, Sascha Hauer пишет:
> > > > > > > On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
> > > > > > > > 25.02.2022 10:51, Sascha Hauer пишет:
> > > > > > > > > The rk3568 HDMI has an additional clock that needs to be
> > > > > > > > > enabled for the
> > > > > > > > > HDMI controller to work. The purpose of that clock is not
> > > > > > > > > clear. It is
> > > > > > > > > named "hclk" in the downstream driver, so use the same name.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Sascha Hauer
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > Notes:
> > > > > > > > > Changes since v5:
> > > > > > > > > - Use devm_clk_get_optional rather than devm_clk_get
> > > > > > > > >
> > > > > > > > >drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
> > > > > > > > >
> > > > > > > > >1 file changed, 16 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > > > > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > > > > index fe4f9556239ac..c6c00e8779ab5 100644
> > > > > > > > > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > > > > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > > > > @@ -76,6 +76,7 @@ struct rockchip_hdmi {
> > > > > > > > > const struct rockchip_hdmi_chip_data *chip_data;
> > > > > > > > > struct clk *ref_clk;
> > > > > > > > > struct clk *grf_clk;
> > > > > > > > > + struct clk *hclk_clk;
> > > > > > > > > struct dw_hdmi *hdmi;
> > > > > > > > > struct regulator *avdd_0v9;
> > > > > > > > > struct regulator *avdd_1v8;
> > > > > > > > > @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct
> > > > > > > > > rockchip_hdmi *hdmi)
> > > > > > > > > return PTR_ERR(hdmi->grf_clk);
> > > > > > > > > }
> > > > > > > > > + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev,
> > > > > > > > > "hclk");
> > > > > > > > > + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
> > > > > > > >
> > > > > > > > Have you tried to investigate the hclk? I'm still thinking
> > > > > > > > that's not
> > > > > > > > only HDMI that needs this clock and then the hardware
> > > > > > > > description
> > > > > > > > doesn't look correct.
> > > > > > >
> > > > > > > I am still not sure what you mean. Yes, it's not only the HDMI
> > > > > > > that
> > > > > > > needs this clock. The VOP2 needs it as well and the driver
> > > > > > > handles that.
> > > > > >
> > > > > > I'm curious whether DSI/DP also need that clock to be enabled. If
> > > > > > they
> > > > > > do, then you aren't modeling h/w properly AFAICS.
> > > > >
> > > > > Assuming nobody at Rockchip decided to make things needlessly
> > > > > inconsistent
> > > > > with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB
> > > > > slave
> > > > > interface. Usually, if that affected anything other than accessing VOP
> > > > > registers, indeed it would smell of something being wrong in the
> > > > > clock tree,
> > > > > but in this case I'd also be suspicious of whether it might have
> > > > > ended up
> > > > > clocking related GRF registers as well (either directly, or
> > > > > indirectly via
> > > > > some gate that the clock driver hasn't modelled yet).
> > > >
> > > > Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
> > > > hanging when HCLK_VOP is disabled by disabling that clock via sysfs
> > > > using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
> > > > of that units can't be accessed. However, when I disable HCLK_VOP by
> > > > directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
> > > > accessing VOP registers hangs, the other units stay functional.
> > > > So it seems it must be the parent clock which must be enabled. The
> > > > parent clock is hclk_vo. This clock should be handled as part of the
> > > > RK3568_PD_VO power domain:
> > > >
> > > > power-domain@RK3568_PD_VO {
> > > > reg = ;
> > > > clocks = <&cru HCLK_VO>,
> > > > <&cru PCLK_VO>,
> > > > <&cru ACLK_VOP_PRE>;
> > > > pm_qos = <&qos_hdcp>,
> > > ><&qos_vop_m0>,
> > > ><&qos_vop_m1>;
> > > > #power-domain-cells = <0>;
> > > > };
> > >
> > > Forget this. The clocks in this node are only enabled during enabling or
> > > disabling the power domain, they are disabled again immediately
> > > afterwards.
>
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On Tue, Mar 01, 2022 at 01:39:31PM +, Robin Murphy wrote:
> On 2022-02-28 14:19, Sascha Hauer wrote:
> > On Fri, Feb 25, 2022 at 02:11:54PM +0100, Sascha Hauer wrote:
> > > On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
> > > > On 2022-02-25 11:10, Dmitry Osipenko wrote:
> > > > > 25.02.2022 13:49, Sascha Hauer пишет:
> > > > > > On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
> > > > > > > 25.02.2022 10:51, Sascha Hauer пишет:
> > > > > > > > The rk3568 HDMI has an additional clock that needs to be
> > > > > > > > enabled for the
> > > > > > > > HDMI controller to work. The purpose of that clock is not
> > > > > > > > clear. It is
> > > > > > > > named "hclk" in the downstream driver, so use the same name.
> > > > > > > >
> > > > > > > > Signed-off-by: Sascha Hauer
> > > > > > > > ---
> > > > > > > >
> > > > > > > > Notes:
> > > > > > > > Changes since v5:
> > > > > > > > - Use devm_clk_get_optional rather than devm_clk_get
> > > > > > > >
> > > > > > > >drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
> > > > > > > >
> > > > > > > >1 file changed, 16 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > > > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > > > index fe4f9556239ac..c6c00e8779ab5 100644
> > > > > > > > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > > > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > > > @@ -76,6 +76,7 @@ struct rockchip_hdmi {
> > > > > > > > const struct rockchip_hdmi_chip_data *chip_data;
> > > > > > > > struct clk *ref_clk;
> > > > > > > > struct clk *grf_clk;
> > > > > > > > + struct clk *hclk_clk;
> > > > > > > > struct dw_hdmi *hdmi;
> > > > > > > > struct regulator *avdd_0v9;
> > > > > > > > struct regulator *avdd_1v8;
> > > > > > > > @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct
> > > > > > > > rockchip_hdmi *hdmi)
> > > > > > > > return PTR_ERR(hdmi->grf_clk);
> > > > > > > > }
> > > > > > > > + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev,
> > > > > > > > "hclk");
> > > > > > > > + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
> > > > > > >
> > > > > > > Have you tried to investigate the hclk? I'm still thinking that's
> > > > > > > not
> > > > > > > only HDMI that needs this clock and then the hardware description
> > > > > > > doesn't look correct.
> > > > > >
> > > > > > I am still not sure what you mean. Yes, it's not only the HDMI that
> > > > > > needs this clock. The VOP2 needs it as well and the driver handles
> > > > > > that.
> > > > >
> > > > > I'm curious whether DSI/DP also need that clock to be enabled. If they
> > > > > do, then you aren't modeling h/w properly AFAICS.
> > > >
> > > > Assuming nobody at Rockchip decided to make things needlessly
> > > > inconsistent
> > > > with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
> > > > interface. Usually, if that affected anything other than accessing VOP
> > > > registers, indeed it would smell of something being wrong in the clock
> > > > tree,
> > > > but in this case I'd also be suspicious of whether it might have ended
> > > > up
> > > > clocking related GRF registers as well (either directly, or indirectly
> > > > via
> > > > some gate that the clock driver hasn't modelled yet).
> > >
> > > Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
> > > hanging when HCLK_VOP is disabled by disabling that clock via sysfs
> > > using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
> > > of that units can't be accessed. However, when I disable HCLK_VOP by
> > > directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
> > > accessing VOP registers hangs, the other units stay functional.
> > > So it seems it must be the parent clock which must be enabled. The
> > > parent clock is hclk_vo. This clock should be handled as part of the
> > > RK3568_PD_VO power domain:
> > >
> > > power-domain@RK3568_PD_VO {
> > > reg = ;
> > > clocks = <&cru HCLK_VO>,
> > > <&cru PCLK_VO>,
> > > <&cru ACLK_VOP_PRE>;
> > > pm_qos = <&qos_hdcp>,
> > ><&qos_vop_m0>,
> > ><&qos_vop_m1>;
> > > #power-domain-cells = <0>;
> > > };
> >
> > Forget this. The clocks in this node are only enabled during enabling or
> > disabling the power domain, they are disabled again immediately afterwards.
> >
> > OK, I need HCLK_VO to access the HDMI registers. I verified that by
> > disabling HCLK_VO at register level (CRU_GATE_CON(20) BIT(1)). The
> > HDMI registers become inaccessible then. This means I'll replace
> > HCLK_VOP in the HDMI node with HCLK_VO. Does this sound sane?
>
> Well, it'
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On 2022-02-28 14:19, Sascha Hauer wrote:
On Fri, Feb 25, 2022 at 02:11:54PM +0100, Sascha Hauer wrote:
On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
On 2022-02-25 11:10, Dmitry Osipenko wrote:
25.02.2022 13:49, Sascha Hauer пишет:
On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
25.02.2022 10:51, Sascha Hauer пишет:
The rk3568 HDMI has an additional clock that needs to be enabled for the
HDMI controller to work. The purpose of that clock is not clear. It is
named "hclk" in the downstream driver, so use the same name.
Signed-off-by: Sascha Hauer
---
Notes:
Changes since v5:
- Use devm_clk_get_optional rather than devm_clk_get
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index fe4f9556239ac..c6c00e8779ab5 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -76,6 +76,7 @@ struct rockchip_hdmi {
const struct rockchip_hdmi_chip_data *chip_data;
struct clk *ref_clk;
struct clk *grf_clk;
+ struct clk *hclk_clk;
struct dw_hdmi *hdmi;
struct regulator *avdd_0v9;
struct regulator *avdd_1v8;
@@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
*hdmi)
return PTR_ERR(hdmi->grf_clk);
}
+ hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
+ if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
Have you tried to investigate the hclk? I'm still thinking that's not
only HDMI that needs this clock and then the hardware description
doesn't look correct.
I am still not sure what you mean. Yes, it's not only the HDMI that
needs this clock. The VOP2 needs it as well and the driver handles that.
I'm curious whether DSI/DP also need that clock to be enabled. If they
do, then you aren't modeling h/w properly AFAICS.
Assuming nobody at Rockchip decided to make things needlessly inconsistent
with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
interface. Usually, if that affected anything other than accessing VOP
registers, indeed it would smell of something being wrong in the clock tree,
but in this case I'd also be suspicious of whether it might have ended up
clocking related GRF registers as well (either directly, or indirectly via
some gate that the clock driver hasn't modelled yet).
Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
hanging when HCLK_VOP is disabled by disabling that clock via sysfs
using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
of that units can't be accessed. However, when I disable HCLK_VOP by
directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
accessing VOP registers hangs, the other units stay functional.
So it seems it must be the parent clock which must be enabled. The
parent clock is hclk_vo. This clock should be handled as part of the
RK3568_PD_VO power domain:
power-domain@RK3568_PD_VO {
reg = ;
clocks = <&cru HCLK_VO>,
<&cru PCLK_VO>,
<&cru ACLK_VOP_PRE>;
pm_qos = <&qos_hdcp>,
<&qos_vop_m0>,
<&qos_vop_m1>;
#power-domain-cells = <0>;
};
Forget this. The clocks in this node are only enabled during enabling or
disabling the power domain, they are disabled again immediately afterwards.
OK, I need HCLK_VO to access the HDMI registers. I verified that by
disabling HCLK_VO at register level (CRU_GATE_CON(20) BIT(1)). The
HDMI registers become inaccessible then. This means I'll replace
HCLK_VOP in the HDMI node with HCLK_VO. Does this sound sane?
Well, it's still a mystery hack overall, and in some ways it seems even
more suspect to be claiming a whole branch of the clock tree rather than
a leaf gate with a specific purpose. I'm really starting to think that
the underlying issue here is a bug in the clock driver, or a hardware
mishap that should logically be worked around by the clock driver,
rather than individual the consumers.
Does it work if you hack the clock driver to think that PCLK_VO is a
child of HCLK_VO? Even if that's not technically true, it would seem to
effectively match the observed behaviour (i.e. all 3 things whose
register access apparently *should* be enabled by a gate off PCLK_VO,
seem to also require HCLK_VO).
Thanks,
Robin.
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On 3/1/22 11:37, Sascha Hauer wrote:
> On Tue, Mar 01, 2022 at 01:56:59AM +0300, Dmitry Osipenko wrote:
>> On 2/28/22 17:19, Sascha Hauer wrote:
>>> On Fri, Feb 25, 2022 at 02:11:54PM +0100, Sascha Hauer wrote:
On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
> On 2022-02-25 11:10, Dmitry Osipenko wrote:
>> 25.02.2022 13:49, Sascha Hauer пишет:
>>> On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
25.02.2022 10:51, Sascha Hauer пишет:
> The rk3568 HDMI has an additional clock that needs to be enabled for
> the
> HDMI controller to work. The purpose of that clock is not clear. It is
> named "hclk" in the downstream driver, so use the same name.
>
> Signed-off-by: Sascha Hauer
> ---
>
> Notes:
> Changes since v5:
> - Use devm_clk_get_optional rather than devm_clk_get
>
> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index fe4f9556239ac..c6c00e8779ab5 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -76,6 +76,7 @@ struct rockchip_hdmi {
> const struct rockchip_hdmi_chip_data *chip_data;
> struct clk *ref_clk;
> struct clk *grf_clk;
> + struct clk *hclk_clk;
> struct dw_hdmi *hdmi;
> struct regulator *avdd_0v9;
> struct regulator *avdd_1v8;
> @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct
> rockchip_hdmi *hdmi)
> return PTR_ERR(hdmi->grf_clk);
> }
> + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
> + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
Have you tried to investigate the hclk? I'm still thinking that's not
only HDMI that needs this clock and then the hardware description
doesn't look correct.
>>>
>>> I am still not sure what you mean. Yes, it's not only the HDMI that
>>> needs this clock. The VOP2 needs it as well and the driver handles that.
>>
>> I'm curious whether DSI/DP also need that clock to be enabled. If they
>> do, then you aren't modeling h/w properly AFAICS.
>
> Assuming nobody at Rockchip decided to make things needlessly inconsistent
> with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
> interface. Usually, if that affected anything other than accessing VOP
> registers, indeed it would smell of something being wrong in the clock
> tree,
> but in this case I'd also be suspicious of whether it might have ended up
> clocking related GRF registers as well (either directly, or indirectly via
> some gate that the clock driver hasn't modelled yet).
Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
hanging when HCLK_VOP is disabled by disabling that clock via sysfs
using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
of that units can't be accessed. However, when I disable HCLK_VOP by
directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
accessing VOP registers hangs, the other units stay functional.
So it seems it must be the parent clock which must be enabled. The
parent clock is hclk_vo. This clock should be handled as part of the
RK3568_PD_VO power domain:
power-domain@RK3568_PD_VO {
reg = ;
clocks = <&cru HCLK_VO>,
<&cru PCLK_VO>,
<&cru ACLK_VOP_PRE>;
pm_qos = <&qos_hdcp>,
<&qos_vop_m0>,
<&qos_vop_m1>;
#power-domain-cells = <0>;
};
>>>
>>> Forget this. The clocks in this node are only enabled during enabling or
>>> disabling the power domain, they are disabled again immediately afterwards.
>>>
>>> OK, I need HCLK_VO to access the HDMI registers. I verified that by
>>> disabling HCLK_VO at register level (CRU_GATE_CON(20) BIT(1)). The
>>> HDMI registers become inaccessible then. This means I'll replace
>>> HCLK_VOP in the HDMI node with HCLK_VO. Does this sound sane?
>>
>> The RK3568_PD_VO already has HCLK_VO and the domain should be
>> auto-enabled before HDMI registers are accessed,
>
> As said, the clocks given in the power domain are only enabled during
> the process of enabling/disabling the power domain and are disabled
> again directly afterwards:
>
>> if (rockchip_pmu_domain_is_on(pd) != power_on) {
>
> They are enabled here:
>
>> ret = clk_bulk_enable(pd
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On Tue, Mar 01, 2022 at 01:56:59AM +0300, Dmitry Osipenko wrote:
> On 2/28/22 17:19, Sascha Hauer wrote:
> > On Fri, Feb 25, 2022 at 02:11:54PM +0100, Sascha Hauer wrote:
> >> On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
> >>> On 2022-02-25 11:10, Dmitry Osipenko wrote:
> 25.02.2022 13:49, Sascha Hauer пишет:
> > On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
> >> 25.02.2022 10:51, Sascha Hauer пишет:
> >>> The rk3568 HDMI has an additional clock that needs to be enabled for
> >>> the
> >>> HDMI controller to work. The purpose of that clock is not clear. It is
> >>> named "hclk" in the downstream driver, so use the same name.
> >>>
> >>> Signed-off-by: Sascha Hauer
> >>> ---
> >>>
> >>> Notes:
> >>> Changes since v5:
> >>> - Use devm_clk_get_optional rather than devm_clk_get
> >>>
> >>> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
> >>> 1 file changed, 16 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> >>> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> >>> index fe4f9556239ac..c6c00e8779ab5 100644
> >>> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> >>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> >>> @@ -76,6 +76,7 @@ struct rockchip_hdmi {
> >>> const struct rockchip_hdmi_chip_data *chip_data;
> >>> struct clk *ref_clk;
> >>> struct clk *grf_clk;
> >>> + struct clk *hclk_clk;
> >>> struct dw_hdmi *hdmi;
> >>> struct regulator *avdd_0v9;
> >>> struct regulator *avdd_1v8;
> >>> @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct
> >>> rockchip_hdmi *hdmi)
> >>> return PTR_ERR(hdmi->grf_clk);
> >>> }
> >>> + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
> >>> + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
> >>
> >> Have you tried to investigate the hclk? I'm still thinking that's not
> >> only HDMI that needs this clock and then the hardware description
> >> doesn't look correct.
> >
> > I am still not sure what you mean. Yes, it's not only the HDMI that
> > needs this clock. The VOP2 needs it as well and the driver handles that.
>
> I'm curious whether DSI/DP also need that clock to be enabled. If they
> do, then you aren't modeling h/w properly AFAICS.
> >>>
> >>> Assuming nobody at Rockchip decided to make things needlessly inconsistent
> >>> with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
> >>> interface. Usually, if that affected anything other than accessing VOP
> >>> registers, indeed it would smell of something being wrong in the clock
> >>> tree,
> >>> but in this case I'd also be suspicious of whether it might have ended up
> >>> clocking related GRF registers as well (either directly, or indirectly via
> >>> some gate that the clock driver hasn't modelled yet).
> >>
> >> Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
> >> hanging when HCLK_VOP is disabled by disabling that clock via sysfs
> >> using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
> >> of that units can't be accessed. However, when I disable HCLK_VOP by
> >> directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
> >> accessing VOP registers hangs, the other units stay functional.
> >> So it seems it must be the parent clock which must be enabled. The
> >> parent clock is hclk_vo. This clock should be handled as part of the
> >> RK3568_PD_VO power domain:
> >>
> >>power-domain@RK3568_PD_VO {
> >> reg = ;
> >> clocks = <&cru HCLK_VO>,
> >> <&cru PCLK_VO>,
> >> <&cru ACLK_VOP_PRE>;
> >> pm_qos = <&qos_hdcp>,
> >> <&qos_vop_m0>,
> >> <&qos_vop_m1>;
> >> #power-domain-cells = <0>;
> >> };
> >
> > Forget this. The clocks in this node are only enabled during enabling or
> > disabling the power domain, they are disabled again immediately afterwards.
> >
> > OK, I need HCLK_VO to access the HDMI registers. I verified that by
> > disabling HCLK_VO at register level (CRU_GATE_CON(20) BIT(1)). The
> > HDMI registers become inaccessible then. This means I'll replace
> > HCLK_VOP in the HDMI node with HCLK_VO. Does this sound sane?
>
> The RK3568_PD_VO already has HCLK_VO and the domain should be
> auto-enabled before HDMI registers are accessed,
As said, the clocks given in the power domain are only enabled during
the process of enabling/disabling the power domain and are disabled
again directly afterwards:
> if (rockchip_pmu_domain_is_on(pd) != power_on) {
They are enabled here:
> ret = clk_bulk_enable(pd->num_clks, pd->clks);
> if (ret < 0) {
>
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On 2/28/22 17:19, Sascha Hauer wrote:
> On Fri, Feb 25, 2022 at 02:11:54PM +0100, Sascha Hauer wrote:
>> On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
>>> On 2022-02-25 11:10, Dmitry Osipenko wrote:
25.02.2022 13:49, Sascha Hauer пишет:
> On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
>> 25.02.2022 10:51, Sascha Hauer пишет:
>>> The rk3568 HDMI has an additional clock that needs to be enabled for the
>>> HDMI controller to work. The purpose of that clock is not clear. It is
>>> named "hclk" in the downstream driver, so use the same name.
>>>
>>> Signed-off-by: Sascha Hauer
>>> ---
>>>
>>> Notes:
>>> Changes since v5:
>>> - Use devm_clk_get_optional rather than devm_clk_get
>>>
>>> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> index fe4f9556239ac..c6c00e8779ab5 100644
>>> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> @@ -76,6 +76,7 @@ struct rockchip_hdmi {
>>> const struct rockchip_hdmi_chip_data *chip_data;
>>> struct clk *ref_clk;
>>> struct clk *grf_clk;
>>> + struct clk *hclk_clk;
>>> struct dw_hdmi *hdmi;
>>> struct regulator *avdd_0v9;
>>> struct regulator *avdd_1v8;
>>> @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct
>>> rockchip_hdmi *hdmi)
>>> return PTR_ERR(hdmi->grf_clk);
>>> }
>>> + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
>>> + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
>>
>> Have you tried to investigate the hclk? I'm still thinking that's not
>> only HDMI that needs this clock and then the hardware description
>> doesn't look correct.
>
> I am still not sure what you mean. Yes, it's not only the HDMI that
> needs this clock. The VOP2 needs it as well and the driver handles that.
I'm curious whether DSI/DP also need that clock to be enabled. If they
do, then you aren't modeling h/w properly AFAICS.
>>>
>>> Assuming nobody at Rockchip decided to make things needlessly inconsistent
>>> with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
>>> interface. Usually, if that affected anything other than accessing VOP
>>> registers, indeed it would smell of something being wrong in the clock tree,
>>> but in this case I'd also be suspicious of whether it might have ended up
>>> clocking related GRF registers as well (either directly, or indirectly via
>>> some gate that the clock driver hasn't modelled yet).
>>
>> Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
>> hanging when HCLK_VOP is disabled by disabling that clock via sysfs
>> using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
>> of that units can't be accessed. However, when I disable HCLK_VOP by
>> directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
>> accessing VOP registers hangs, the other units stay functional.
>> So it seems it must be the parent clock which must be enabled. The
>> parent clock is hclk_vo. This clock should be handled as part of the
>> RK3568_PD_VO power domain:
>>
>> power-domain@RK3568_PD_VO {
>> reg = ;
>> clocks = <&cru HCLK_VO>,
>> <&cru PCLK_VO>,
>> <&cru ACLK_VOP_PRE>;
>> pm_qos = <&qos_hdcp>,
>> <&qos_vop_m0>,
>> <&qos_vop_m1>;
>> #power-domain-cells = <0>;
>> };
>
> Forget this. The clocks in this node are only enabled during enabling or
> disabling the power domain, they are disabled again immediately afterwards.
>
> OK, I need HCLK_VO to access the HDMI registers. I verified that by
> disabling HCLK_VO at register level (CRU_GATE_CON(20) BIT(1)). The
> HDMI registers become inaccessible then. This means I'll replace
> HCLK_VOP in the HDMI node with HCLK_VO. Does this sound sane?
The RK3568_PD_VO already has HCLK_VO and the domain should be
auto-enabled before HDMI registers are accessed, hence you should do the
opposite and remove the HCLK_VO/P clock from the HDMI DT, not add it. If
the HCLK_VO clock isn't enabled by the domain driver, then you need to
check why. Or am I missing something?
What about DSI and DP? Don't they depend on RK3568_PD_VO as well?
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On Fri, Feb 25, 2022 at 02:11:54PM +0100, Sascha Hauer wrote:
> On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
> > On 2022-02-25 11:10, Dmitry Osipenko wrote:
> > > 25.02.2022 13:49, Sascha Hauer пишет:
> > > > On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
> > > > > 25.02.2022 10:51, Sascha Hauer пишет:
> > > > > > The rk3568 HDMI has an additional clock that needs to be enabled
> > > > > > for the
> > > > > > HDMI controller to work. The purpose of that clock is not clear. It
> > > > > > is
> > > > > > named "hclk" in the downstream driver, so use the same name.
> > > > > >
> > > > > > Signed-off-by: Sascha Hauer
> > > > > > ---
> > > > > >
> > > > > > Notes:
> > > > > > Changes since v5:
> > > > > > - Use devm_clk_get_optional rather than devm_clk_get
> > > > > >
> > > > > > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
> > > > > > 1 file changed, 16 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > index fe4f9556239ac..c6c00e8779ab5 100644
> > > > > > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > > @@ -76,6 +76,7 @@ struct rockchip_hdmi {
> > > > > > const struct rockchip_hdmi_chip_data *chip_data;
> > > > > > struct clk *ref_clk;
> > > > > > struct clk *grf_clk;
> > > > > > + struct clk *hclk_clk;
> > > > > > struct dw_hdmi *hdmi;
> > > > > > struct regulator *avdd_0v9;
> > > > > > struct regulator *avdd_1v8;
> > > > > > @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct
> > > > > > rockchip_hdmi *hdmi)
> > > > > > return PTR_ERR(hdmi->grf_clk);
> > > > > > }
> > > > > > + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
> > > > > > + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
> > > > >
> > > > > Have you tried to investigate the hclk? I'm still thinking that's not
> > > > > only HDMI that needs this clock and then the hardware description
> > > > > doesn't look correct.
> > > >
> > > > I am still not sure what you mean. Yes, it's not only the HDMI that
> > > > needs this clock. The VOP2 needs it as well and the driver handles that.
> > >
> > > I'm curious whether DSI/DP also need that clock to be enabled. If they
> > > do, then you aren't modeling h/w properly AFAICS.
> >
> > Assuming nobody at Rockchip decided to make things needlessly inconsistent
> > with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
> > interface. Usually, if that affected anything other than accessing VOP
> > registers, indeed it would smell of something being wrong in the clock tree,
> > but in this case I'd also be suspicious of whether it might have ended up
> > clocking related GRF registers as well (either directly, or indirectly via
> > some gate that the clock driver hasn't modelled yet).
>
> Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
> hanging when HCLK_VOP is disabled by disabling that clock via sysfs
> using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
> of that units can't be accessed. However, when I disable HCLK_VOP by
> directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
> accessing VOP registers hangs, the other units stay functional.
> So it seems it must be the parent clock which must be enabled. The
> parent clock is hclk_vo. This clock should be handled as part of the
> RK3568_PD_VO power domain:
>
> power-domain@RK3568_PD_VO {
> reg = ;
> clocks = <&cru HCLK_VO>,
> <&cru PCLK_VO>,
> <&cru ACLK_VOP_PRE>;
> pm_qos = <&qos_hdcp>,
> <&qos_vop_m0>,
> <&qos_vop_m1>;
> #power-domain-cells = <0>;
> };
Forget this. The clocks in this node are only enabled during enabling or
disabling the power domain, they are disabled again immediately afterwards.
OK, I need HCLK_VO to access the HDMI registers. I verified that by
disabling HCLK_VO at register level (CRU_GATE_CON(20) BIT(1)). The
HDMI registers become inaccessible then. This means I'll replace
HCLK_VOP in the HDMI node with HCLK_VO. Does this sound sane?
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On 2022-02-25 13:11, Sascha Hauer wrote:
On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
On 2022-02-25 11:10, Dmitry Osipenko wrote:
25.02.2022 13:49, Sascha Hauer пишет:
On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
25.02.2022 10:51, Sascha Hauer пишет:
The rk3568 HDMI has an additional clock that needs to be enabled for the
HDMI controller to work. The purpose of that clock is not clear. It is
named "hclk" in the downstream driver, so use the same name.
Signed-off-by: Sascha Hauer
---
Notes:
Changes since v5:
- Use devm_clk_get_optional rather than devm_clk_get
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index fe4f9556239ac..c6c00e8779ab5 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -76,6 +76,7 @@ struct rockchip_hdmi {
const struct rockchip_hdmi_chip_data *chip_data;
struct clk *ref_clk;
struct clk *grf_clk;
+ struct clk *hclk_clk;
struct dw_hdmi *hdmi;
struct regulator *avdd_0v9;
struct regulator *avdd_1v8;
@@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
*hdmi)
return PTR_ERR(hdmi->grf_clk);
}
+ hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
+ if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
Have you tried to investigate the hclk? I'm still thinking that's not
only HDMI that needs this clock and then the hardware description
doesn't look correct.
I am still not sure what you mean. Yes, it's not only the HDMI that
needs this clock. The VOP2 needs it as well and the driver handles that.
I'm curious whether DSI/DP also need that clock to be enabled. If they
do, then you aren't modeling h/w properly AFAICS.
Assuming nobody at Rockchip decided to make things needlessly inconsistent
with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
interface. Usually, if that affected anything other than accessing VOP
registers, indeed it would smell of something being wrong in the clock tree,
but in this case I'd also be suspicious of whether it might have ended up
clocking related GRF registers as well (either directly, or indirectly via
some gate that the clock driver hasn't modelled yet).
Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
hanging when HCLK_VOP is disabled by disabling that clock via sysfs
using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
of that units can't be accessed. However, when I disable HCLK_VOP by
directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
accessing VOP registers hangs, the other units stay functional.
So it seems it must be the parent clock which must be enabled. The
parent clock is hclk_vo. This clock should be handled as part of the
RK3568_PD_VO power domain:
power-domain@RK3568_PD_VO {
reg = ;
clocks = <&cru HCLK_VO>,
<&cru PCLK_VO>,
<&cru ACLK_VOP_PRE>;
pm_qos = <&qos_hdcp>,
<&qos_vop_m0>,
<&qos_vop_m1>;
#power-domain-cells = <0>;
};
The HDMI controller is part of that domain, so I think this should work,
but it doesn't. That's where I am now, I'll have a closer look.
Ah, interesting. Looking at the clock driver, I'd also be suspicious
whether pclk_vo is somehow messed up such that we're currently relying
on hclk_vo to keep the common grandparent enabled. Seems like the DSI
and eDP (and HDCP if anyone ever used it) registers would be similarly
affected if so, and sure enough they both have a similarly suspect extra
"hclk" in the downstream DT too.
Robin.
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On Fri, Feb 25, 2022 at 12:41:23PM +, Robin Murphy wrote:
> On 2022-02-25 11:10, Dmitry Osipenko wrote:
> > 25.02.2022 13:49, Sascha Hauer пишет:
> > > On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
> > > > 25.02.2022 10:51, Sascha Hauer пишет:
> > > > > The rk3568 HDMI has an additional clock that needs to be enabled for
> > > > > the
> > > > > HDMI controller to work. The purpose of that clock is not clear. It is
> > > > > named "hclk" in the downstream driver, so use the same name.
> > > > >
> > > > > Signed-off-by: Sascha Hauer
> > > > > ---
> > > > >
> > > > > Notes:
> > > > > Changes since v5:
> > > > > - Use devm_clk_get_optional rather than devm_clk_get
> > > > >
> > > > > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
> > > > > 1 file changed, 16 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > index fe4f9556239ac..c6c00e8779ab5 100644
> > > > > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > > > @@ -76,6 +76,7 @@ struct rockchip_hdmi {
> > > > > const struct rockchip_hdmi_chip_data *chip_data;
> > > > > struct clk *ref_clk;
> > > > > struct clk *grf_clk;
> > > > > + struct clk *hclk_clk;
> > > > > struct dw_hdmi *hdmi;
> > > > > struct regulator *avdd_0v9;
> > > > > struct regulator *avdd_1v8;
> > > > > @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct
> > > > > rockchip_hdmi *hdmi)
> > > > > return PTR_ERR(hdmi->grf_clk);
> > > > > }
> > > > > + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
> > > > > + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
> > > >
> > > > Have you tried to investigate the hclk? I'm still thinking that's not
> > > > only HDMI that needs this clock and then the hardware description
> > > > doesn't look correct.
> > >
> > > I am still not sure what you mean. Yes, it's not only the HDMI that
> > > needs this clock. The VOP2 needs it as well and the driver handles that.
> >
> > I'm curious whether DSI/DP also need that clock to be enabled. If they
> > do, then you aren't modeling h/w properly AFAICS.
>
> Assuming nobody at Rockchip decided to make things needlessly inconsistent
> with previous SoCs, HCLK_VOP should be the clock for the VOP's AHB slave
> interface. Usually, if that affected anything other than accessing VOP
> registers, indeed it would smell of something being wrong in the clock tree,
> but in this case I'd also be suspicious of whether it might have ended up
> clocking related GRF registers as well (either directly, or indirectly via
> some gate that the clock driver hasn't modelled yet).
Ok, I am beginning to understand. I verified that hdmi, mipi and dp are
hanging when HCLK_VOP is disabled by disabling that clock via sysfs
using CLOCK_ALLOW_WRITE_DEBUGFS. When it's disabled then the registers
of that units can't be accessed. However, when I disable HCLK_VOP by
directly writing to the gate bit RK3568_CLKGATE_CON(20) then only
accessing VOP registers hangs, the other units stay functional.
So it seems it must be the parent clock which must be enabled. The
parent clock is hclk_vo. This clock should be handled as part of the
RK3568_PD_VO power domain:
power-domain@RK3568_PD_VO {
reg = ;
clocks = <&cru HCLK_VO>,
<&cru PCLK_VO>,
<&cru ACLK_VOP_PRE>;
pm_qos = <&qos_hdcp>,
<&qos_vop_m0>,
<&qos_vop_m1>;
#power-domain-cells = <0>;
};
The HDMI controller is part of that domain, so I think this should work,
but it doesn't. That's where I am now, I'll have a closer look.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On 2022-02-25 11:10, Dmitry Osipenko wrote:
25.02.2022 13:49, Sascha Hauer пишет:
On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
25.02.2022 10:51, Sascha Hauer пишет:
The rk3568 HDMI has an additional clock that needs to be enabled for the
HDMI controller to work. The purpose of that clock is not clear. It is
named "hclk" in the downstream driver, so use the same name.
Signed-off-by: Sascha Hauer
---
Notes:
Changes since v5:
- Use devm_clk_get_optional rather than devm_clk_get
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index fe4f9556239ac..c6c00e8779ab5 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -76,6 +76,7 @@ struct rockchip_hdmi {
const struct rockchip_hdmi_chip_data *chip_data;
struct clk *ref_clk;
struct clk *grf_clk;
+ struct clk *hclk_clk;
struct dw_hdmi *hdmi;
struct regulator *avdd_0v9;
struct regulator *avdd_1v8;
@@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
*hdmi)
return PTR_ERR(hdmi->grf_clk);
}
+ hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
+ if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
Have you tried to investigate the hclk? I'm still thinking that's not
only HDMI that needs this clock and then the hardware description
doesn't look correct.
I am still not sure what you mean. Yes, it's not only the HDMI that
needs this clock. The VOP2 needs it as well and the driver handles that.
I'm curious whether DSI/DP also need that clock to be enabled. If they
do, then you aren't modeling h/w properly AFAICS.
Assuming nobody at Rockchip decided to make things needlessly
inconsistent with previous SoCs, HCLK_VOP should be the clock for the
VOP's AHB slave interface. Usually, if that affected anything other than
accessing VOP registers, indeed it would smell of something being wrong
in the clock tree, but in this case I'd also be suspicious of whether it
might have ended up clocking related GRF registers as well (either
directly, or indirectly via some gate that the clock driver hasn't
modelled yet).
If the symptom of not claiming HCLK_VOP is hanging on some register
access in the HDMI driver while the VOP is idle, then it should be
relatively straightforward to narrow down with some logging, and see if
it looks like this is really just another "grf" clock. If not, then
we're back to suspecting something more insidiously wrong elsewhere.
Robin.
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On Fri, Feb 25, 2022 at 02:10:55PM +0300, Dmitry Osipenko wrote:
> 25.02.2022 13:49, Sascha Hauer пишет:
> > On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
> >> 25.02.2022 10:51, Sascha Hauer пишет:
> >>> The rk3568 HDMI has an additional clock that needs to be enabled for the
> >>> HDMI controller to work. The purpose of that clock is not clear. It is
> >>> named "hclk" in the downstream driver, so use the same name.
> >>>
> >>> Signed-off-by: Sascha Hauer
> >>> ---
> >>>
> >>> Notes:
> >>> Changes since v5:
> >>> - Use devm_clk_get_optional rather than devm_clk_get
> >>>
> >>> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
> >>> 1 file changed, 16 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> >>> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> >>> index fe4f9556239ac..c6c00e8779ab5 100644
> >>> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> >>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> >>> @@ -76,6 +76,7 @@ struct rockchip_hdmi {
> >>> const struct rockchip_hdmi_chip_data *chip_data;
> >>> struct clk *ref_clk;
> >>> struct clk *grf_clk;
> >>> + struct clk *hclk_clk;
> >>> struct dw_hdmi *hdmi;
> >>> struct regulator *avdd_0v9;
> >>> struct regulator *avdd_1v8;
> >>> @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct
> >>> rockchip_hdmi *hdmi)
> >>> return PTR_ERR(hdmi->grf_clk);
> >>> }
> >>>
> >>> + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
> >>> + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
> >>
> >> Have you tried to investigate the hclk? I'm still thinking that's not
> >> only HDMI that needs this clock and then the hardware description
> >> doesn't look correct.
> >
> > I am still not sure what you mean. Yes, it's not only the HDMI that
> > needs this clock. The VOP2 needs it as well and the driver handles that.
>
> I'm curious whether DSI/DP also need that clock to be enabled. If they
> do, then you aren't modeling h/w properly AFAICS.
Indeed I can confirm that DSI and DP need that clock enabled for
register access as well. Do you think these devices should be under an
additional bus layer in the device tree which drives the clock? Or
should HCLK_VOP be enabled as part of the RK3568_PD_VO power domain?
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
25.02.2022 13:49, Sascha Hauer пишет:
> On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
>> 25.02.2022 10:51, Sascha Hauer пишет:
>>> The rk3568 HDMI has an additional clock that needs to be enabled for the
>>> HDMI controller to work. The purpose of that clock is not clear. It is
>>> named "hclk" in the downstream driver, so use the same name.
>>>
>>> Signed-off-by: Sascha Hauer
>>> ---
>>>
>>> Notes:
>>> Changes since v5:
>>> - Use devm_clk_get_optional rather than devm_clk_get
>>>
>>> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> index fe4f9556239ac..c6c00e8779ab5 100644
>>> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> @@ -76,6 +76,7 @@ struct rockchip_hdmi {
>>> const struct rockchip_hdmi_chip_data *chip_data;
>>> struct clk *ref_clk;
>>> struct clk *grf_clk;
>>> + struct clk *hclk_clk;
>>> struct dw_hdmi *hdmi;
>>> struct regulator *avdd_0v9;
>>> struct regulator *avdd_1v8;
>>> @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
>>> *hdmi)
>>> return PTR_ERR(hdmi->grf_clk);
>>> }
>>>
>>> + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
>>> + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
>>
>> Have you tried to investigate the hclk? I'm still thinking that's not
>> only HDMI that needs this clock and then the hardware description
>> doesn't look correct.
>
> I am still not sure what you mean. Yes, it's not only the HDMI that
> needs this clock. The VOP2 needs it as well and the driver handles that.
I'm curious whether DSI/DP also need that clock to be enabled. If they
do, then you aren't modeling h/w properly AFAICS.
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
On Fri, Feb 25, 2022 at 01:26:14PM +0300, Dmitry Osipenko wrote:
> 25.02.2022 10:51, Sascha Hauer пишет:
> > The rk3568 HDMI has an additional clock that needs to be enabled for the
> > HDMI controller to work. The purpose of that clock is not clear. It is
> > named "hclk" in the downstream driver, so use the same name.
> >
> > Signed-off-by: Sascha Hauer
> > ---
> >
> > Notes:
> > Changes since v5:
> > - Use devm_clk_get_optional rather than devm_clk_get
> >
> > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > index fe4f9556239ac..c6c00e8779ab5 100644
> > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > @@ -76,6 +76,7 @@ struct rockchip_hdmi {
> > const struct rockchip_hdmi_chip_data *chip_data;
> > struct clk *ref_clk;
> > struct clk *grf_clk;
> > + struct clk *hclk_clk;
> > struct dw_hdmi *hdmi;
> > struct regulator *avdd_0v9;
> > struct regulator *avdd_1v8;
> > @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
> > *hdmi)
> > return PTR_ERR(hdmi->grf_clk);
> > }
> >
> > + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
> > + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
>
> Have you tried to investigate the hclk? I'm still thinking that's not
> only HDMI that needs this clock and then the hardware description
> doesn't look correct.
I am still not sure what you mean. Yes, it's not only the HDMI that
needs this clock. The VOP2 needs it as well and the driver handles that.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v7 10/24] drm/rockchip: dw_hdmi: Add support for hclk
25.02.2022 10:51, Sascha Hauer пишет:
> The rk3568 HDMI has an additional clock that needs to be enabled for the
> HDMI controller to work. The purpose of that clock is not clear. It is
> named "hclk" in the downstream driver, so use the same name.
>
> Signed-off-by: Sascha Hauer
> ---
>
> Notes:
> Changes since v5:
> - Use devm_clk_get_optional rather than devm_clk_get
>
> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index fe4f9556239ac..c6c00e8779ab5 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -76,6 +76,7 @@ struct rockchip_hdmi {
> const struct rockchip_hdmi_chip_data *chip_data;
> struct clk *ref_clk;
> struct clk *grf_clk;
> + struct clk *hclk_clk;
> struct dw_hdmi *hdmi;
> struct regulator *avdd_0v9;
> struct regulator *avdd_1v8;
> @@ -229,6 +230,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
> *hdmi)
> return PTR_ERR(hdmi->grf_clk);
> }
>
> + hdmi->hclk_clk = devm_clk_get_optional(hdmi->dev, "hclk");
> + if (PTR_ERR(hdmi->hclk_clk) == -EPROBE_DEFER) {
Have you tried to investigate the hclk? I'm still thinking that's not
only HDMI that needs this clock and then the hardware description
doesn't look correct.
