| Message ID | 20230118193015.911074-10-markyacoub@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/hdcp: Pull HDCP auth/exchange/check into helpers | expand |
Hi Mark On 1/18/2023 11:30 AM, Mark Yacoub wrote: > From: Sean Paul <seanpaul@chromium.org> > > 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 <seanpaul@chromium.org> > Signed-off-by: Mark Yacoub <markyacoub@chromium.org> > Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-sean@poorly.run #v1 > Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-sean@poorly.run #v2 > Link: https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-14-sean@poorly.run #v3 > Link: https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-14-sean@poorly.run #v4 > Link: https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-10-sean@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 @@ &mdss_dp { > pinctrl-names = "default"; > pinctrl-0 = <&dp_hot_plug_det>; > data-lanes = <0 1>; > + > + reg = <0 0x0ae90000 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? > > &pm6150_adc {
On Wed, 18 Jan 2023 at 21:30, Mark Yacoub <markyacoub@chromium.org> wrote: > > From: Sean Paul <seanpaul@chromium.org> > > 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 <seanpaul@chromium.org> > Signed-off-by: Mark Yacoub <markyacoub@chromium.org> > Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-sean@poorly.run #v1 > Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-sean@poorly.run #v2 > Link: https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-14-sean@poorly.run #v3 > Link: https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-14-sean@poorly.run #v4 > Link: https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-10-sean@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 @@ &mdss_dp { > pinctrl-names = "default"; > pinctrl-0 = <&dp_hot_plug_det>; > data-lanes = <0 1>; > + > + reg = <0 0x0ae90000 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? > }; > > &pm6150_adc { > -- > 2.39.0.246.g2a6d74b583-goog >
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 <seanpaul@chromium.org> > > > > 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 <seanpaul@chromium.org> > > Signed-off-by: Mark Yacoub <markyacoub@chromium.org> > > Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-sean@poorly.run #v1 > > Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-sean@poorly.run #v2 > > Link: https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-14-sean@poorly.run #v3 > > Link: https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-14-sean@poorly.run #v4 > > Link: https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-10-sean@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 >
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 <seanpaul@chromium.org> >>> >>> 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 <seanpaul@chromium.org> >>> Signed-off-by: Mark Yacoub <markyacoub@chromium.org> >>> Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-sean@poorly.run #v1 >>> Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-sean@poorly.run #v2 >>> Link: https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-14-sean@poorly.run #v3 >>> Link: https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-14-sean@poorly.run #v4 >>> Link: https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-10-sean@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.
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 @@ &mdss_dp { pinctrl-names = "default"; pinctrl-0 = <&dp_hot_plug_det>; data-lanes = <0 1>; + + reg = <0 0x0ae90000 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>; }; &pm6150_adc {