> -----Original Message-----
> From: Sean Paul [mailto:seanp...@chromium.org]
> Sent: Thursday, July 12, 2018 2:36 AM
> To: C, Ramalingam <ramalinga...@intel.com>
> Cc: Sean Paul <seanp...@chromium.org>; intel-...@lists.freedesktop.org; dri-
> de...@lists.freedesktop.org; dan...@ffwll.ch; Winkler, Tomas
> <tomas.wink...@intel.com>; Usyskin, Alexander
> <alexander.usys...@intel.com>; Shankar, Uma <uma.shan...@intel.com>
> Subject: Re: [PATCH v5 09/40] drm/i915: Schedule hdcp_check_link in
> _intel_hdcp_enable
>
> On Wed, Jul 11, 2018 at 07:07:10PM +0000, C, Ramalingam wrote:
> >
> > > -----Original Message-----
> > > From: Sean Paul [mailto:seanp...@chromium.org]
> > > Sent: Tuesday, July 10, 2018 2:04 AM
> > > To: C, Ramalingam <ramalinga...@intel.com>
> > > Cc: intel-...@lists.freedesktop.org;
> > > dri-devel@lists.freedesktop.org; dan...@ffwll.ch; Winkler, Tomas
> > > <tomas.wink...@intel.com>; Usyskin, Alexander
> > > <alexander.usys...@intel.com>; Shankar, Uma <uma.shan...@intel.com>
> > > Subject: Re: [PATCH v5 09/40] drm/i915: Schedule hdcp_check_link in
> > > _intel_hdcp_enable
> > >
> > > On Wed, Jun 27, 2018 at 02:09:58PM +0530, Ramalingam C wrote:
> > > > As a preparation for making the intel_hdcp_enable as common
> > > > function for both HDCP1.4 and HDCP2.2, HDCP1.4 check_link
> > > > scheduling is moved into _intel_hdcp_enable() function.
> > > >
> > > > v3:
> > > > No Changes.
> > > > v4:
> > > > Style fix.
> > > > v5:
> > > > No Change.
> > > >
> > > > Signed-off-by: Ramalingam C <ramalinga...@intel.com>
> > > > Reviewed-by: Uma Shankar <uma.shan...@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_hdcp.c | 12 ++++++++----
> > > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c
> > > > b/drivers/gpu/drm/i915/intel_hdcp.c
> > > > index 769560591aa8..4bff74b3bed0 100644
> > > > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > > > @@ -688,7 +688,7 @@ static int _intel_hdcp_enable(struct
> > > > intel_connector
> > > *connector)
> > > > ret = intel_hdcp_auth(conn_to_dig_port(connector),
> > > > hdcp->hdcp_shim);
> > > > if (!ret)
> > > > - return 0;
> > > > + break;
> > > >
> > > > DRM_DEBUG_KMS("HDCP Auth failure (%d)\n", ret);
> > > >
> > > > @@ -696,7 +696,13 @@ static int _intel_hdcp_enable(struct
> > > > intel_connector
> > > *connector)
> > > > _intel_hdcp_disable(connector);
> > > > }
> > > >
> > > > - DRM_ERROR("HDCP authentication failed (%d tries/%d)\n", tries,
> > > > ret);
> > > > + if (i != tries)
> > > > + schedule_delayed_work(&hdcp->hdcp_check_work,
> > > > + DRM_HDCP_CHECK_PERIOD_MS);
> > >
> > > At best, this results in a duplicate scheduling when called from
> > > intel_hdcp_check_link(). At worst, it schedules a check when it's
> > > not supposed to (see the condition in intel_hdcp_check_work).
> >
> > In original HDCP1.4 implementation hdcp_check_work is scheduled at
> intel_hdcp_enable.
> > To make the intel_hdcp_enable as a common function for 1.4 and 2.2,
> > the same hdcp_check_work is scheduled at _intel_hdcp_enable which is
> enable function for HDCP1.4.
> >
> > So no real functional change with this patch. If you are mentioning
> > the check_work is getting scheduled before prop_work and/or hdcp_value
> update, check_link handles that.
> > Means if the hdcp value is not UNDESIRED then shim's link_check will be
> invoked.
> >
> > Hope I understood your concern right.
>
> I'm not sure. intel_hdcp_check_link() calls _intel_hdcp_enable(), which is now
> scheduling a check. So, in essence, intel_hdcp_check_work() is rescheduling
> itself through this call chain. Once intel_hdcp_check_link() returns in
> intel_hdcp_check_work(), the worker reschedules itself if the link is good,
> so you
> get a duplicate schedule.
Oops! Thanks for catching me there. I missed to notice that _intel_hdcp_enable
is called from check_link also.
I will fix that within intel_hdcp_enable itself. In that case this patch is no
more needed.
Thanks,
Ram
>
> Sean
>
> >
> > Thanks,
> > Ram
> > >
> > > Sean
> > >
> > > > + else
> > > > + DRM_ERROR("HDCP authentication failed (%d tries/%d)\n",
> > > > + tries, ret);
> > > > +
> > > > return ret;
> > > > }
> > > >
> > > > @@ -790,8 +796,6 @@ int intel_hdcp_enable(struct intel_connector
> > > > *connector)
> > > >
> > > > hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED;
> > > > schedule_work(&hdcp->hdcp_prop_work);
> > > > - schedule_delayed_work(&hdcp->hdcp_check_work,
> > > > - DRM_HDCP_CHECK_PERIOD_MS);
> > > > out:
> > > > mutex_unlock(&hdcp->hdcp_mutex);
> > > > return ret;
> > > > --
> > > > 2.7.4
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> > > --
> > > Sean Paul, Software Engineer, Google / Chromium OS
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
