| Message ID | 20230118193015.911074-10-markyacoub@google.com |
|---|---|
| State | New |
| 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 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.
On 20/01/2023 16: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? What do you mean "better things to do"? I review the patches as that's expected from maintainer. I spend a lot of time on so indeed I could find some other ways to use it. I spot something which is obvious mistake, although trivial and not important, but clicking automated answer is also trivial for me and fast. And yes. 90% of my reviews answers are automated because people cannot learn to test before sending, cannot learn to read Submitting Patches and many other trivial things. Clicking this automated answer was also trivial from my point, but entire discussion including your disagreement about incorrect title (read Submitting Patches) instead of just implementing it - is waste of time. I'll then consider not wasting time on your patches. Best regards, Krzysztof
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 {