From: Daniel Vetter <daniel@ffwll.ch> To: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie>, Anshuman Gupta <anshuman.gupta@intel.com>, "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>, Sean Paul <seanpaul@chromium.org>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org> Subject: Re: [Intel-gfx] [PATCH] drm/i915/hdcp: Disable the QSES check for HDCP 1.4 over MST Date: Wed, 13 Jan 2021 21:45:05 +0100 [thread overview] Message-ID: <CAKMK7uH6_krnCkx=Qn1aR3vrr-EC-1ZZVX9USt5uiSpbc_UW-A@mail.gmail.com> (raw) In-Reply-To: <CAMavQKKHKaWnGOg_dRZ-nYO1GrhEjYT8sxxcFwXpcD0Aym0APQ@mail.gmail.com> On Wed, Jan 13, 2021 at 7:34 PM Sean Paul <sean@poorly.run> wrote: > > On Wed, Jan 13, 2021 at 9:31 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Wed, Jan 13, 2021 at 2:39 PM Sean Paul <sean@poorly.run> wrote: > > > > > > On Wed, Jan 13, 2021 at 5:34 AM Anshuman Gupta <anshuman.gupta@intel.com> wrote: > > > > > > > > On 2021-01-07 at 04:08:58 +0530, Sean Paul wrote: > > > > > From: Sean Paul <seanpaul@chromium.org> > > > > > > > > > > The HDCP 1.4 spec does not require the QUERY_STREAM_ENCRYPTION_STATUS > > > > IMHO DP 1.4 vesa specs I.3.5 mark QSES as desirale for both HDCP 1.4 and HDCP 2.2. > > > > "The MST Source device may use a QUERY_STREAM_ENCRYPTION_STATUS message > > > > transaction to query the downstream status for a particular stream." > > > > > > > > I feel it useful for scenario in which a non hdcp supported monitor > > > > is hot plugged to MST branch. Source really doesn't know about the hdcp > > > > capable device on MST branch, it just know the capability of immediate > > > > downstream device. QSES can fetch the HDCP capability from MST topology. > > > > We don't require to enable stream encryption for such streams. > > > > > > I agree it's useful when it works, but unfortunately it's broken on at > > > least 2 MST bridge chips I've encountered :/ > > > > > > Until we can figure out a) how to fix them (ie: firmware updates), or > > > b) how to enumerate all of the broken chips to create quirks, we > > > probably just want to disable QSES for HDCP 1.4. > > > > What happens when the user plugs in a non-hdcp screen into a hub which > > doesn't do QSES? Just black screen? > > > > Good question, thanks for forcing me to explain myself more thoroughly :) > > This patch doesn't change that behavior, QSES is currently only used > as a means for verifying the stream continues to be encrypted in > steady-state (ie: after auth has already completed and the pixels are > flowing). > > If one wanted to check HDCP 1.4 capability upfront, QSES wouldn't be > the way to do it. Instead you would tunnel a remote DPCD to the sink > to read the BCAPS register (ie: the same way we check non-MST > connectors). > > So QSES is currently only around in HDCP 1.4 as an extra precaution > against a bug in the code preventing the MST stream from being > encrypted. IMO broken HW overrules suspenders when we already have a > belt :) I think with the above explanation added to the commit message this makes sense to merge. Fwiw, i.e. not much: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cheers, Daniel > > > Sean > > > That would suck a bit, otoh with broken hw I don't see how we could do > > better :-/ > > -Daniel > > > > > Sean > > > > > > > > check, it was always a nice-to-have. After deploying this across various > > > > > devices, we've determined that some MST bridge chips do not properly > > > > > support this call for HDCP 1.4 (namely Synaptics and Realtek). > > > > > > > > > > I had considered creating a quirk for this, but I think it's more > > > > > prudent to just disable the check entirely since I don't have an idea > > > > > how widespread support is. > > > > May be we can remove it from the link check and can retain as utility ? > > > > Thanks, > > > > Anshuman Gupta. > > > > > > > > > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 26 +------------------- > > > > > 1 file changed, 1 insertion(+), 25 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > > > index 03424d20e9f7..b6a9606bf09a 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > > > @@ -640,30 +640,6 @@ intel_dp_mst_hdcp_toggle_signalling(struct intel_digital_port *dig_port, > > > > > return ret; > > > > > } > > > > > > > > > > -static > > > > > -bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port, > > > > > - struct intel_connector *connector) > > > > > -{ > > > > > - struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > > > > > - struct intel_dp *intel_dp = &dig_port->dp; > > > > > - struct drm_dp_query_stream_enc_status_ack_reply reply; > > > > > - int ret; > > > > > - > > > > > - if (!intel_dp_hdcp_check_link(dig_port, connector)) > > > > > - return false; > > > > > - > > > > > - ret = drm_dp_send_query_stream_enc_status(&intel_dp->mst_mgr, > > > > > - connector->port, &reply); > > > > > - if (ret) { > > > > > - drm_dbg_kms(&i915->drm, > > > > > - "[CONNECTOR:%d:%s] failed QSES ret=%d\n", > > > > > - connector->base.base.id, connector->base.name, ret); > > > > > - return false; > > > > > - } > > > > > - > > > > > - return reply.auth_completed && reply.encryption_enabled; > > > > > -} > > > > > - > > > > > static const struct intel_hdcp_shim intel_dp_mst_hdcp_shim = { > > > > > .write_an_aksv = intel_dp_hdcp_write_an_aksv, > > > > > .read_bksv = intel_dp_hdcp_read_bksv, > > > > > @@ -674,7 +650,7 @@ static const struct intel_hdcp_shim intel_dp_mst_hdcp_shim = { > > > > > .read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo, > > > > > .read_v_prime_part = intel_dp_hdcp_read_v_prime_part, > > > > > .toggle_signalling = intel_dp_mst_hdcp_toggle_signalling, > > > > > - .check_link = intel_dp_mst_hdcp_check_link, > > > > > + .check_link = intel_dp_hdcp_check_link, > > > > > .hdcp_capable = intel_dp_hdcp_capable, > > > > > > > > > > .protocol = HDCP_PROTOCOL_DP, > > > > > -- > > > > > Sean Paul, Software Engineer, Google / Chromium OS > > > > > > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch> To: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie>, "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>, Sean Paul <seanpaul@chromium.org>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org> Subject: Re: [Intel-gfx] [PATCH] drm/i915/hdcp: Disable the QSES check for HDCP 1.4 over MST Date: Wed, 13 Jan 2021 21:45:05 +0100 [thread overview] Message-ID: <CAKMK7uH6_krnCkx=Qn1aR3vrr-EC-1ZZVX9USt5uiSpbc_UW-A@mail.gmail.com> (raw) In-Reply-To: <CAMavQKKHKaWnGOg_dRZ-nYO1GrhEjYT8sxxcFwXpcD0Aym0APQ@mail.gmail.com> On Wed, Jan 13, 2021 at 7:34 PM Sean Paul <sean@poorly.run> wrote: > > On Wed, Jan 13, 2021 at 9:31 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Wed, Jan 13, 2021 at 2:39 PM Sean Paul <sean@poorly.run> wrote: > > > > > > On Wed, Jan 13, 2021 at 5:34 AM Anshuman Gupta <anshuman.gupta@intel.com> wrote: > > > > > > > > On 2021-01-07 at 04:08:58 +0530, Sean Paul wrote: > > > > > From: Sean Paul <seanpaul@chromium.org> > > > > > > > > > > The HDCP 1.4 spec does not require the QUERY_STREAM_ENCRYPTION_STATUS > > > > IMHO DP 1.4 vesa specs I.3.5 mark QSES as desirale for both HDCP 1.4 and HDCP 2.2. > > > > "The MST Source device may use a QUERY_STREAM_ENCRYPTION_STATUS message > > > > transaction to query the downstream status for a particular stream." > > > > > > > > I feel it useful for scenario in which a non hdcp supported monitor > > > > is hot plugged to MST branch. Source really doesn't know about the hdcp > > > > capable device on MST branch, it just know the capability of immediate > > > > downstream device. QSES can fetch the HDCP capability from MST topology. > > > > We don't require to enable stream encryption for such streams. > > > > > > I agree it's useful when it works, but unfortunately it's broken on at > > > least 2 MST bridge chips I've encountered :/ > > > > > > Until we can figure out a) how to fix them (ie: firmware updates), or > > > b) how to enumerate all of the broken chips to create quirks, we > > > probably just want to disable QSES for HDCP 1.4. > > > > What happens when the user plugs in a non-hdcp screen into a hub which > > doesn't do QSES? Just black screen? > > > > Good question, thanks for forcing me to explain myself more thoroughly :) > > This patch doesn't change that behavior, QSES is currently only used > as a means for verifying the stream continues to be encrypted in > steady-state (ie: after auth has already completed and the pixels are > flowing). > > If one wanted to check HDCP 1.4 capability upfront, QSES wouldn't be > the way to do it. Instead you would tunnel a remote DPCD to the sink > to read the BCAPS register (ie: the same way we check non-MST > connectors). > > So QSES is currently only around in HDCP 1.4 as an extra precaution > against a bug in the code preventing the MST stream from being > encrypted. IMO broken HW overrules suspenders when we already have a > belt :) I think with the above explanation added to the commit message this makes sense to merge. Fwiw, i.e. not much: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cheers, Daniel > > > Sean > > > That would suck a bit, otoh with broken hw I don't see how we could do > > better :-/ > > -Daniel > > > > > Sean > > > > > > > > check, it was always a nice-to-have. After deploying this across various > > > > > devices, we've determined that some MST bridge chips do not properly > > > > > support this call for HDCP 1.4 (namely Synaptics and Realtek). > > > > > > > > > > I had considered creating a quirk for this, but I think it's more > > > > > prudent to just disable the check entirely since I don't have an idea > > > > > how widespread support is. > > > > May be we can remove it from the link check and can retain as utility ? > > > > Thanks, > > > > Anshuman Gupta. > > > > > > > > > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 26 +------------------- > > > > > 1 file changed, 1 insertion(+), 25 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > > > index 03424d20e9f7..b6a9606bf09a 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > > > @@ -640,30 +640,6 @@ intel_dp_mst_hdcp_toggle_signalling(struct intel_digital_port *dig_port, > > > > > return ret; > > > > > } > > > > > > > > > > -static > > > > > -bool intel_dp_mst_hdcp_check_link(struct intel_digital_port *dig_port, > > > > > - struct intel_connector *connector) > > > > > -{ > > > > > - struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > > > > > - struct intel_dp *intel_dp = &dig_port->dp; > > > > > - struct drm_dp_query_stream_enc_status_ack_reply reply; > > > > > - int ret; > > > > > - > > > > > - if (!intel_dp_hdcp_check_link(dig_port, connector)) > > > > > - return false; > > > > > - > > > > > - ret = drm_dp_send_query_stream_enc_status(&intel_dp->mst_mgr, > > > > > - connector->port, &reply); > > > > > - if (ret) { > > > > > - drm_dbg_kms(&i915->drm, > > > > > - "[CONNECTOR:%d:%s] failed QSES ret=%d\n", > > > > > - connector->base.base.id, connector->base.name, ret); > > > > > - return false; > > > > > - } > > > > > - > > > > > - return reply.auth_completed && reply.encryption_enabled; > > > > > -} > > > > > - > > > > > static const struct intel_hdcp_shim intel_dp_mst_hdcp_shim = { > > > > > .write_an_aksv = intel_dp_hdcp_write_an_aksv, > > > > > .read_bksv = intel_dp_hdcp_read_bksv, > > > > > @@ -674,7 +650,7 @@ static const struct intel_hdcp_shim intel_dp_mst_hdcp_shim = { > > > > > .read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo, > > > > > .read_v_prime_part = intel_dp_hdcp_read_v_prime_part, > > > > > .toggle_signalling = intel_dp_mst_hdcp_toggle_signalling, > > > > > - .check_link = intel_dp_mst_hdcp_check_link, > > > > > + .check_link = intel_dp_hdcp_check_link, > > > > > .hdcp_capable = intel_dp_hdcp_capable, > > > > > > > > > > .protocol = HDCP_PROTOCOL_DP, > > > > > -- > > > > > Sean Paul, Software Engineer, Google / Chromium OS > > > > > > > > > > _______________________________________________ > > > > > Intel-gfx mailing list > > > > > Intel-gfx@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2021-01-13 20:45 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-06 22:38 [PATCH] drm/i915/hdcp: Disable the QSES check for HDCP 1.4 over MST Sean Paul 2021-01-06 22:38 ` [Intel-gfx] " Sean Paul 2021-01-06 23:41 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork 2021-01-07 10:13 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2021-01-12 17:51 ` [PATCH] " Jani Nikula 2021-01-12 17:51 ` [Intel-gfx] " Jani Nikula 2021-01-13 10:19 ` Anshuman Gupta 2021-01-13 10:19 ` Anshuman Gupta 2021-01-13 13:39 ` Sean Paul 2021-01-13 13:39 ` Sean Paul 2021-01-13 14:31 ` Daniel Vetter 2021-01-13 14:31 ` Daniel Vetter 2021-01-13 18:33 ` Sean Paul 2021-01-13 18:33 ` Sean Paul 2021-01-13 20:45 ` Daniel Vetter [this message] 2021-01-13 20:45 ` Daniel Vetter 2021-01-15 10:22 ` Gupta, Anshuman 2021-01-15 10:22 ` Gupta, Anshuman 2021-01-21 17:26 ` [PATCH v2] " Sean Paul 2021-01-21 17:26 ` [Intel-gfx] " Sean Paul 2021-01-22 2:42 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/hdcp: Disable the QSES check for HDCP 1.4 over MST (rev2) Patchwork 2021-01-25 15:26 ` Gupta, Anshuman 2021-01-25 16:41 ` Vudum, Lakshminarayana 2021-01-25 17:10 ` Vudum, Lakshminarayana 2021-01-27 5:20 ` Gupta, Anshuman 2021-01-27 5:29 ` Vudum, Lakshminarayana 2021-01-27 7:37 ` Gupta, Anshuman 2021-01-27 19:13 ` Vudum, Lakshminarayana 2021-01-28 7:09 ` Petri Latvala 2021-01-25 17:02 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2021-01-28 7:05 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork 2021-01-28 7:22 ` Gupta, Anshuman
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='CAKMK7uH6_krnCkx=Qn1aR3vrr-EC-1ZZVX9USt5uiSpbc_UW-A@mail.gmail.com' \ --to=daniel@ffwll.ch \ --cc=airlied@linux.ie \ --cc=anshuman.gupta@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=sean@poorly.run \ --cc=seanpaul@chromium.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: linkBe 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.