From: "Heiko Stübner" <heiko@sntech.de>
To: Benjamin Gaignard <benjamin.gaignard@collabora.com>,
hjc@rock-chips.com, airlied@linux.ie, daniel@ffwll.ch,
robh+dt@kernel.org, algea.cao@rock-chips.com,
andy.yan@rock-chips.com,
Michael Riesch <michael.riesch@wolfvision.net>
Cc: dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
kernel@collabora.com
Subject: Re: [PATCH v2 1/2] dt-bindings: display: rockchip: Add compatible for rk3568 HDMI
Date: Tue, 13 Jul 2021 10:49:37 +0200 [thread overview]
Message-ID: <3865833.Ac65pObt5d@diego> (raw)
In-Reply-To: <1bd64284-0a20-12e3-e2e7-19cdfdbf1a25@wolfvision.net>
Hi Michael,
Am Dienstag, 13. Juli 2021, 10:44:00 CEST schrieb Michael Riesch:
> The HDMI TX block in the RK3568 requires two power supplies, which have
> to be enabled in some cases (at least on the RK3568 EVB1 the voltages
> VDDA0V9_IMAGE and VCCA1V8_IMAGE are disabled by default). It would be
> great if this was considered by the driver and the device tree binding.
> I am not sure, though, whether this is a RK3568 specific or
> rockchip_dw_hdmi specific thing. Maybe it can even enter the Synopsis DW
> HDMI driver.
I do remember that this discussion happened many years back already.
And yes the supplies are needed for all but back then there was opposition
as these are supposedly phy-related supplies, not for the dw-hdmi itself.
[There are variants with an external phy, like on the rk3328]
See discussion on [0]
[0] https://dri-devel.freedesktop.narkive.com/pen2zWo1/patch-v3-1-2-drm-bridge-dw-hdmi-support-optional-supply-regulators
> On 7/7/21 2:03 PM, Benjamin Gaignard wrote:
> > Define a new compatible for rk3568 HDMI.
> > This version of HDMI hardware block needs two new clocks hclk_vio and hclk
> > to provide phy reference clocks.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > ---
> > version 2:
> > - Add the clocks needed for the phy.
> >
> > .../bindings/display/rockchip/rockchip,dw-hdmi.yaml | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > index 75cd9c686e985..cb8643b3a8b84 100644
> > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > @@ -23,6 +23,7 @@ properties:
> > - rockchip,rk3288-dw-hdmi
> > - rockchip,rk3328-dw-hdmi
> > - rockchip,rk3399-dw-hdmi
> > + - rockchip,rk3568-dw-hdmi
> >
> > reg-io-width:
> > const: 4
> > @@ -51,8 +52,11 @@ properties:
> > - vpll
> > - enum:
> > - grf
> > + - hclk_vio
> > + - vpll
> > + - enum:
> > + - hclk
> > - vpll
> > - - const: vpll
>
> The description and documentation of the clocks are somewhat misleading
> IMHO. This is not caused by your patches, of course. But maybe this is a
> chance to clean them up a bit.
>
> It seems that the CEC clock is an optional clock of the dw-hdmi driver.
> Shouldn't it be documented in the synopsys,dw-hdmi.yaml?
>
> Also, it would be nice if the clocks hclk_vio and hclk featured a
> description in the binding.
>
> BTW, I am not too familiar with the syntax here, but shouldn't items in
> clocks and items in clock-names be aligned (currently, there is a plain
> list vs. an enum structure)?
>
> Best regards,
> Michael
>
> >
> > ddc-i2c-bus:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
>
WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: Benjamin Gaignard <benjamin.gaignard@collabora.com>,
hjc@rock-chips.com, airlied@linux.ie, daniel@ffwll.ch,
robh+dt@kernel.org, algea.cao@rock-chips.com,
andy.yan@rock-chips.com,
Michael Riesch <michael.riesch@wolfvision.net>
Cc: dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
kernel@collabora.com
Subject: Re: [PATCH v2 1/2] dt-bindings: display: rockchip: Add compatible for rk3568 HDMI
Date: Tue, 13 Jul 2021 10:49:37 +0200 [thread overview]
Message-ID: <3865833.Ac65pObt5d@diego> (raw)
In-Reply-To: <1bd64284-0a20-12e3-e2e7-19cdfdbf1a25@wolfvision.net>
Hi Michael,
Am Dienstag, 13. Juli 2021, 10:44:00 CEST schrieb Michael Riesch:
> The HDMI TX block in the RK3568 requires two power supplies, which have
> to be enabled in some cases (at least on the RK3568 EVB1 the voltages
> VDDA0V9_IMAGE and VCCA1V8_IMAGE are disabled by default). It would be
> great if this was considered by the driver and the device tree binding.
> I am not sure, though, whether this is a RK3568 specific or
> rockchip_dw_hdmi specific thing. Maybe it can even enter the Synopsis DW
> HDMI driver.
I do remember that this discussion happened many years back already.
And yes the supplies are needed for all but back then there was opposition
as these are supposedly phy-related supplies, not for the dw-hdmi itself.
[There are variants with an external phy, like on the rk3328]
See discussion on [0]
[0] https://dri-devel.freedesktop.narkive.com/pen2zWo1/patch-v3-1-2-drm-bridge-dw-hdmi-support-optional-supply-regulators
> On 7/7/21 2:03 PM, Benjamin Gaignard wrote:
> > Define a new compatible for rk3568 HDMI.
> > This version of HDMI hardware block needs two new clocks hclk_vio and hclk
> > to provide phy reference clocks.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > ---
> > version 2:
> > - Add the clocks needed for the phy.
> >
> > .../bindings/display/rockchip/rockchip,dw-hdmi.yaml | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > index 75cd9c686e985..cb8643b3a8b84 100644
> > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > @@ -23,6 +23,7 @@ properties:
> > - rockchip,rk3288-dw-hdmi
> > - rockchip,rk3328-dw-hdmi
> > - rockchip,rk3399-dw-hdmi
> > + - rockchip,rk3568-dw-hdmi
> >
> > reg-io-width:
> > const: 4
> > @@ -51,8 +52,11 @@ properties:
> > - vpll
> > - enum:
> > - grf
> > + - hclk_vio
> > + - vpll
> > + - enum:
> > + - hclk
> > - vpll
> > - - const: vpll
>
> The description and documentation of the clocks are somewhat misleading
> IMHO. This is not caused by your patches, of course. But maybe this is a
> chance to clean them up a bit.
>
> It seems that the CEC clock is an optional clock of the dw-hdmi driver.
> Shouldn't it be documented in the synopsys,dw-hdmi.yaml?
>
> Also, it would be nice if the clocks hclk_vio and hclk featured a
> description in the binding.
>
> BTW, I am not too familiar with the syntax here, but shouldn't items in
> clocks and items in clock-names be aligned (currently, there is a plain
> list vs. an enum structure)?
>
> Best regards,
> Michael
>
> >
> > ddc-i2c-bus:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: Benjamin Gaignard <benjamin.gaignard@collabora.com>,
hjc@rock-chips.com, airlied@linux.ie, daniel@ffwll.ch,
robh+dt@kernel.org, algea.cao@rock-chips.com,
andy.yan@rock-chips.com,
Michael Riesch <michael.riesch@wolfvision.net>
Cc: dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
kernel@collabora.com
Subject: Re: [PATCH v2 1/2] dt-bindings: display: rockchip: Add compatible for rk3568 HDMI
Date: Tue, 13 Jul 2021 10:49:37 +0200 [thread overview]
Message-ID: <3865833.Ac65pObt5d@diego> (raw)
In-Reply-To: <1bd64284-0a20-12e3-e2e7-19cdfdbf1a25@wolfvision.net>
Hi Michael,
Am Dienstag, 13. Juli 2021, 10:44:00 CEST schrieb Michael Riesch:
> The HDMI TX block in the RK3568 requires two power supplies, which have
> to be enabled in some cases (at least on the RK3568 EVB1 the voltages
> VDDA0V9_IMAGE and VCCA1V8_IMAGE are disabled by default). It would be
> great if this was considered by the driver and the device tree binding.
> I am not sure, though, whether this is a RK3568 specific or
> rockchip_dw_hdmi specific thing. Maybe it can even enter the Synopsis DW
> HDMI driver.
I do remember that this discussion happened many years back already.
And yes the supplies are needed for all but back then there was opposition
as these are supposedly phy-related supplies, not for the dw-hdmi itself.
[There are variants with an external phy, like on the rk3328]
See discussion on [0]
[0] https://dri-devel.freedesktop.narkive.com/pen2zWo1/patch-v3-1-2-drm-bridge-dw-hdmi-support-optional-supply-regulators
> On 7/7/21 2:03 PM, Benjamin Gaignard wrote:
> > Define a new compatible for rk3568 HDMI.
> > This version of HDMI hardware block needs two new clocks hclk_vio and hclk
> > to provide phy reference clocks.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > ---
> > version 2:
> > - Add the clocks needed for the phy.
> >
> > .../bindings/display/rockchip/rockchip,dw-hdmi.yaml | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > index 75cd9c686e985..cb8643b3a8b84 100644
> > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > @@ -23,6 +23,7 @@ properties:
> > - rockchip,rk3288-dw-hdmi
> > - rockchip,rk3328-dw-hdmi
> > - rockchip,rk3399-dw-hdmi
> > + - rockchip,rk3568-dw-hdmi
> >
> > reg-io-width:
> > const: 4
> > @@ -51,8 +52,11 @@ properties:
> > - vpll
> > - enum:
> > - grf
> > + - hclk_vio
> > + - vpll
> > + - enum:
> > + - hclk
> > - vpll
> > - - const: vpll
>
> The description and documentation of the clocks are somewhat misleading
> IMHO. This is not caused by your patches, of course. But maybe this is a
> chance to clean them up a bit.
>
> It seems that the CEC clock is an optional clock of the dw-hdmi driver.
> Shouldn't it be documented in the synopsys,dw-hdmi.yaml?
>
> Also, it would be nice if the clocks hclk_vio and hclk featured a
> description in the binding.
>
> BTW, I am not too familiar with the syntax here, but shouldn't items in
> clocks and items in clock-names be aligned (currently, there is a plain
> list vs. an enum structure)?
>
> Best regards,
> Michael
>
> >
> > ddc-i2c-bus:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: Benjamin Gaignard <benjamin.gaignard@collabora.com>,
hjc@rock-chips.com, airlied@linux.ie, daniel@ffwll.ch,
robh+dt@kernel.org, algea.cao@rock-chips.com,
andy.yan@rock-chips.com,
Michael Riesch <michael.riesch@wolfvision.net>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org,
linux-rockchip@lists.infradead.org, kernel@collabora.com,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 1/2] dt-bindings: display: rockchip: Add compatible for rk3568 HDMI
Date: Tue, 13 Jul 2021 10:49:37 +0200 [thread overview]
Message-ID: <3865833.Ac65pObt5d@diego> (raw)
In-Reply-To: <1bd64284-0a20-12e3-e2e7-19cdfdbf1a25@wolfvision.net>
Hi Michael,
Am Dienstag, 13. Juli 2021, 10:44:00 CEST schrieb Michael Riesch:
> The HDMI TX block in the RK3568 requires two power supplies, which have
> to be enabled in some cases (at least on the RK3568 EVB1 the voltages
> VDDA0V9_IMAGE and VCCA1V8_IMAGE are disabled by default). It would be
> great if this was considered by the driver and the device tree binding.
> I am not sure, though, whether this is a RK3568 specific or
> rockchip_dw_hdmi specific thing. Maybe it can even enter the Synopsis DW
> HDMI driver.
I do remember that this discussion happened many years back already.
And yes the supplies are needed for all but back then there was opposition
as these are supposedly phy-related supplies, not for the dw-hdmi itself.
[There are variants with an external phy, like on the rk3328]
See discussion on [0]
[0] https://dri-devel.freedesktop.narkive.com/pen2zWo1/patch-v3-1-2-drm-bridge-dw-hdmi-support-optional-supply-regulators
> On 7/7/21 2:03 PM, Benjamin Gaignard wrote:
> > Define a new compatible for rk3568 HDMI.
> > This version of HDMI hardware block needs two new clocks hclk_vio and hclk
> > to provide phy reference clocks.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > ---
> > version 2:
> > - Add the clocks needed for the phy.
> >
> > .../bindings/display/rockchip/rockchip,dw-hdmi.yaml | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > index 75cd9c686e985..cb8643b3a8b84 100644
> > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> > @@ -23,6 +23,7 @@ properties:
> > - rockchip,rk3288-dw-hdmi
> > - rockchip,rk3328-dw-hdmi
> > - rockchip,rk3399-dw-hdmi
> > + - rockchip,rk3568-dw-hdmi
> >
> > reg-io-width:
> > const: 4
> > @@ -51,8 +52,11 @@ properties:
> > - vpll
> > - enum:
> > - grf
> > + - hclk_vio
> > + - vpll
> > + - enum:
> > + - hclk
> > - vpll
> > - - const: vpll
>
> The description and documentation of the clocks are somewhat misleading
> IMHO. This is not caused by your patches, of course. But maybe this is a
> chance to clean them up a bit.
>
> It seems that the CEC clock is an optional clock of the dw-hdmi driver.
> Shouldn't it be documented in the synopsys,dw-hdmi.yaml?
>
> Also, it would be nice if the clocks hclk_vio and hclk featured a
> description in the binding.
>
> BTW, I am not too familiar with the syntax here, but shouldn't items in
> clocks and items in clock-names be aligned (currently, there is a plain
> list vs. an enum structure)?
>
> Best regards,
> Michael
>
> >
> > ddc-i2c-bus:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
>
next prev parent reply other threads:[~2021-07-13 8:49 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-07 12:03 [PATCH v2 0/2] Add support of HDMI for rk3568 Benjamin Gaignard
2021-07-07 12:03 ` Benjamin Gaignard
2021-07-07 12:03 ` Benjamin Gaignard
2021-07-07 12:03 ` Benjamin Gaignard
2021-07-07 12:03 ` [PATCH v2 1/2] dt-bindings: display: rockchip: Add compatible for rk3568 HDMI Benjamin Gaignard
2021-07-07 12:03 ` Benjamin Gaignard
2021-07-07 12:03 ` Benjamin Gaignard
2021-07-07 12:03 ` Benjamin Gaignard
2021-07-12 16:22 ` Rob Herring
2021-07-12 16:22 ` Rob Herring
2021-07-12 16:22 ` Rob Herring
2021-07-12 16:22 ` Rob Herring
2021-07-13 8:44 ` Michael Riesch
2021-07-13 8:44 ` Michael Riesch
2021-07-13 8:44 ` Michael Riesch
2021-07-13 8:44 ` Michael Riesch
2021-07-13 8:49 ` Heiko Stübner [this message]
2021-07-13 8:49 ` Heiko Stübner
2021-07-13 8:49 ` Heiko Stübner
2021-07-13 8:49 ` Heiko Stübner
2021-07-14 9:19 ` Michael Riesch
2021-07-14 9:19 ` Michael Riesch
2021-07-14 9:19 ` Michael Riesch
2021-07-14 9:19 ` Michael Riesch
2021-07-14 12:02 ` Robin Murphy
2021-07-14 12:02 ` Robin Murphy
2021-07-14 12:02 ` Robin Murphy
2021-07-14 12:02 ` Robin Murphy
2021-07-13 10:23 ` Robin Murphy
2021-07-13 10:23 ` Robin Murphy
2021-07-13 10:23 ` Robin Murphy
2021-07-13 10:23 ` Robin Murphy
2021-07-07 12:03 ` [PATCH v2 2/2] drm/rockchip: dw_hdmi: add rk3568 support Benjamin Gaignard
2021-07-07 12:03 ` Benjamin Gaignard
2021-07-07 12:03 ` Benjamin Gaignard
2021-07-07 12:03 ` Benjamin Gaignard
2021-07-13 11:40 ` Alex Bee
2021-07-13 11:40 ` Alex Bee
2021-07-13 11:40 ` Alex Bee
2021-07-13 11:40 ` Alex Bee
2021-07-14 1:26 ` Andy Yan
2021-07-14 1:26 ` Andy Yan
2021-07-14 1:26 ` Andy Yan
2021-07-14 1:26 ` Andy Yan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3865833.Ac65pObt5d@diego \
--to=heiko@sntech.de \
--cc=airlied@linux.ie \
--cc=algea.cao@rock-chips.com \
--cc=andy.yan@rock-chips.com \
--cc=benjamin.gaignard@collabora.com \
--cc=daniel@ffwll.ch \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=hjc@rock-chips.com \
--cc=kernel@collabora.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=michael.riesch@wolfvision.net \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.