Re: [PATCH 2/8] drm/i915: Stop encryption for repeater with no sink

Sean Paul
2018-02-02T15:19:54Z

On Fri, Feb 02, 2018 at 08:33:38PM +0530, Ramalingam C wrote:
>
>
> On Friday 02 February 2018 08:18 PM, Sean Paul wrote:
> > On Fri, Feb 02, 2018 at 07:42:47PM +0530, Ramalingam C wrote:
> > >
> > > On Friday 02 February 2018 07:43 PM, Sean Paul wrote:
> > > > On Fri, Feb 02, 2018 at 04:15:14PM +0530, Ramalingam C wrote:
> > > > > If a HDCP repeater is detected with zero hdcp authenticated
> > > > > downstream devices, there are two option as below:
> > > > >
> > > > > 1. Dont continue on second stage authentication. Disable encryption.
> > > > > 2. Continue with second stage authentication excluding the KSV list and
> > > > > continue encryption on success.
> > > > >
> > > > > This patch adopts the option 1.
> > > > It doesn't seem that hard to adopt option 2 and avoid failure. That would result
> > > > in a better experience.
> > > True. Not too much effort for option 2. But I am not seeing any ROI out of
> > > it at this point.
> > > what is the benefit of encrypting if repeater cant use it, as it has 0 sinks
> > > attached to it.
> > > Still do we want option two?
> > Is it possible the repeater itself has video output? I'm also worried about
> > non-compliant displays which may report having a repeater. If these aren't
> > possible, I agree. Just beef up the comment like "If there are no downstream
> > devices, spec requires we disable all encryption. So fail here to ensure hdcp is
> > disabled"
> >
> > Sean
>
> When spec provide an option that we can disable the HDCP encryption incase
> of repeater with device count 0, that means repeaters cant have display.
>
> Display which claims to be a repeater, mostly wont pass the second stage
> authentication meant for repeaters. So we need not worry about them.
>
> Sure as you mentioned i will rephrase the commit message with these informations.

Not just the commit message. Please also update the comment in the code so it's
clear.

Sean

>
> --Ram
>
> >
> > > -Ram
> > > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_hdcp.c | 4 ++--
> > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> > > > > index 0021511ae4d7..182a3c8a4e4a 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > > > > @@ -175,10 +175,10 @@ int intel_hdcp_auth_downstream(struct intel_connector *connector)
> > > > > return -EPERM;
> > > > > }
> > > > > - /* If there are no downstream devices, we're all done. */
> > > > > + /* If there are no downstream devices, we're not encrypting. */
> > > > > num_downstream = DRM_HDCP_NUM_DOWNSTREAM(bstatus[0]);
> > > > > if (num_downstream == 0)
> > > > > - return 0;
> > > > > + return -EINVAL;
> > > > > ksv_fifo = kzalloc(num_downstream * DRM_HDCP_KSV_LEN, GFP_KERNEL);
> > > > > if (!ksv_fifo)
> > > > > --
> > > > > 2.7.4
> > > > >
>

--
Sean Paul, Software Engineer, Google / Chromium OS