[Intel-gfx] [PATCH v5 09/40] drm/i915: Schedule hdcp_check_link in _intel_hdcp_enable

[C, Ramalingam]

> -----Original Message-----
> From: Sean Paul [mailto:seanpaul at chromium.org]
> Sent: Tuesday, July 10, 2018 2:04 AM
> To: C, Ramalingam <ramalingam.c at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
> daniel at ffwll.ch; Winkler, Tomas <tomas.winkler at intel.com>; Usyskin,
> Alexander <alexander.usyskin at intel.com>; Shankar, Uma
> <uma.shankar at 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 <ramalingam.c at intel.com>
> > Reviewed-by: Uma Shankar <uma.shankar at 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.

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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> --
> Sean Paul, Software Engineer, Google / Chromium OS