Re: [Intel-gfx] [PATCH v6 09/10] arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller
On 20/01/2023 17:54, Sean Paul wrote: On Thu, Jan 19, 2023 at 11:35:32AM +0100, Krzysztof Kozlowski wrote: On 18/01/2023 20:30, Mark Yacoub wrote: From: Sean Paul This patch adds the register ranges required for HDCP key injection and Do not use "This commit/patch". https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 This applies to all your patches. Fix it everywhere. My goodness, this is peak bikeshedding. Surely we have better things to do with our time? While I would not enforce this rule if there were no other issues with the commits, Mark will have to cleanup/rework commits anyway, see other review comments. Thus removing/slightly rephrasing a commit message sounds like a minor issue to me. Signed-off-by: Sean Paul Signed-off-by: Mark Yacoub Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-s...@poorly.run #v1 Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-s...@poorly.run #v2 Link: https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-14-s...@poorly.run #v3 Link: https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-14-s...@poorly.run #v4 Link: https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-10-s...@poorly.run #v5 Drop the links. Why? I've always done this, it seems helpful to me? I'd say, if you wish to include them, they belong to the cover letter, not to the per-commit message. Once landed, they will serve no purpose. -- With best wishes Dmitry
Re: [Intel-gfx] [PATCH v6 09/10] arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller
On Thu, Jan 19, 2023 at 11:35:32AM +0100, Krzysztof Kozlowski wrote: > On 18/01/2023 20:30, Mark Yacoub wrote: > > From: Sean Paul > > > > This patch adds the register ranges required for HDCP key injection and > > Do not use "This commit/patch". > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > > This applies to all your patches. Fix it everywhere. My goodness, this is peak bikeshedding. Surely we have better things to do with our time? > > > HDCP TrustZone interaction as described in the dt-bindings for the > > sc7180 dp controller. Now that these are supported, change the > > compatible string to "dp-hdcp". > > What does it mean? Where do you do it? > > > > > Signed-off-by: Sean Paul > > Signed-off-by: Mark Yacoub > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-s...@poorly.run > > #v1 > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-s...@poorly.run > > #v2 > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-14-s...@poorly.run > > #v3 > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-14-s...@poorly.run > > #v4 > > Link: > > https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-10-s...@poorly.run > > #v5 > > Drop the links. Why? I've always done this, it seems helpful to me? > > > > > Changes in v3: > > -Split off into a new patch containing just the dts change (Stephen) > > -Add hdcp compatible string (Stephen) > > Changes in v4: > > -Rebase on Bjorn's multi-dp patchset > > Changes in v5: > > -Put the tz register offsets in trogdor dtsi (Rob C) > > Changes in v6: > > -Rebased: Removed modifications in sc7180.dtsi as it's already upstream > > > > --- > > Changelog after --- . It's common practice in drm subsystem to include this in the commit message. Sean > > > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 8 > > 1 file changed, 8 insertions(+) > > > > Best regards, > Krzysztof > -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [Intel-gfx] [PATCH v6 09/10] arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller
On Wed, 18 Jan 2023 at 21:30, Mark Yacoub wrote:
>
> From: Sean Paul
>
> This patch adds the register ranges required for HDCP key injection and
> HDCP TrustZone interaction as described in the dt-bindings for the
> sc7180 dp controller. Now that these are supported, change the
> compatible string to "dp-hdcp".
No change in compatibles, so patch description should be updated.
>
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-s...@poorly.run
> #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-s...@poorly.run
> #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-14-s...@poorly.run
> #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-14-s...@poorly.run
> #v4
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-10-s...@poorly.run
> #v5
Again, this probably belongs to a cover letter
>
> Changes in v3:
> -Split off into a new patch containing just the dts change (Stephen)
> -Add hdcp compatible string (Stephen)
> Changes in v4:
> -Rebase on Bjorn's multi-dp patchset
> Changes in v5:
> -Put the tz register offsets in trogdor dtsi (Rob C)
> Changes in v6:
> -Rebased: Removed modifications in sc7180.dtsi as it's already upstream
>
> ---
> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> index 178efaaa89ec..6f6fe5cb6563 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -817,6 +817,14 @@ _dp {
> pinctrl-names = "default";
> pinctrl-0 = <_hot_plug_det>;
> data-lanes = <0 1>;
> +
> + reg = <0 0x0ae9 0 0x200>,
> + <0 0x0ae90200 0 0x200>,
> + <0 0x0ae90400 0 0xc00>,
> + <0 0x0ae91000 0 0x400>,
> + <0 0x0ae91400 0 0x400>,
> + <0 0x0aed1000 0 0x175>,
> + <0 0x0aee1000 0 0x2c>;
Is there any reason for adding these to the -trogdor instead of adding
them to the base dtsi? Does hardware differ between the sc7180.dtsi
and sc7180-trogdor.dtsi?
> };
>
> _adc {
> --
> 2.39.0.246.g2a6d74b583-goog
>
--
With best wishes
Dmitry
Re: [Intel-gfx] [PATCH v6 09/10] arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller
Hi Mark
On 1/18/2023 11:30 AM, Mark Yacoub wrote:
From: Sean Paul
This patch adds the register ranges required for HDCP key injection and
HDCP TrustZone interaction as described in the dt-bindings for the
sc7180 dp controller. Now that these are supported, change the
compatible string to "dp-hdcp".
Signed-off-by: Sean Paul
Signed-off-by: Mark Yacoub
Link:
https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-s...@poorly.run
#v1
Link:
https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-s...@poorly.run
#v2
Link:
https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-14-s...@poorly.run
#v3
Link:
https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-14-s...@poorly.run
#v4
Link:
https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-10-s...@poorly.run
#v5
Changes in v3:
-Split off into a new patch containing just the dts change (Stephen)
-Add hdcp compatible string (Stephen)
Changes in v4:
-Rebase on Bjorn's multi-dp patchset
Changes in v5:
-Put the tz register offsets in trogdor dtsi (Rob C)
Changes in v6:
-Rebased: Removed modifications in sc7180.dtsi as it's already upstream
---
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 178efaaa89ec..6f6fe5cb6563 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -817,6 +817,14 @@ _dp {
pinctrl-names = "default";
pinctrl-0 = <_hot_plug_det>;
data-lanes = <0 1>;
+
+ reg = <0 0x0ae9 0 0x200>,
+ <0 0x0ae90200 0 0x200>,
+ <0 0x0ae90400 0 0xc00>,
+ <0 0x0ae91000 0 0x400>,
+ <0 0x0ae91400 0 0x400>,
+ <0 0x0aed1000 0 0x175>,
+ <0 0x0aee1000 0 0x2c>;
};
Can you pls point me to which tree you rebased this on top of?
The mdss_dp node looks different here:
https://gitlab.com/linux-kernel/linux-next/-/blob/master/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi#L815
For the TZ regs the second entry is fine but any reason for the size of
the first register space to be 0x175 instead of 0x174?
_adc {
[Intel-gfx] [PATCH v6 09/10] arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller
From: Sean Paul
This patch adds the register ranges required for HDCP key injection and
HDCP TrustZone interaction as described in the dt-bindings for the
sc7180 dp controller. Now that these are supported, change the
compatible string to "dp-hdcp".
Signed-off-by: Sean Paul
Signed-off-by: Mark Yacoub
Link:
https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-s...@poorly.run
#v1
Link:
https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-s...@poorly.run
#v2
Link:
https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-14-s...@poorly.run
#v3
Link:
https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-14-s...@poorly.run
#v4
Link:
https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-10-s...@poorly.run
#v5
Changes in v3:
-Split off into a new patch containing just the dts change (Stephen)
-Add hdcp compatible string (Stephen)
Changes in v4:
-Rebase on Bjorn's multi-dp patchset
Changes in v5:
-Put the tz register offsets in trogdor dtsi (Rob C)
Changes in v6:
-Rebased: Removed modifications in sc7180.dtsi as it's already upstream
---
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 178efaaa89ec..6f6fe5cb6563 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -817,6 +817,14 @@ _dp {
pinctrl-names = "default";
pinctrl-0 = <_hot_plug_det>;
data-lanes = <0 1>;
+
+ reg = <0 0x0ae9 0 0x200>,
+ <0 0x0ae90200 0 0x200>,
+ <0 0x0ae90400 0 0xc00>,
+ <0 0x0ae91000 0 0x400>,
+ <0 0x0ae91400 0 0x400>,
+ <0 0x0aed1000 0 0x175>,
+ <0 0x0aee1000 0 0x2c>;
};
_adc {
--
2.39.0.246.g2a6d74b583-goog
