[Intel-gfx] [PATCH v2 3/4] drm/i915: Check hdcp key loadability

[Ville Syrjälä]

On Mon, Apr 02, 2018 at 02:35:42PM +0530, Ramalingam C wrote:
> 
> 
> On Thursday 29 March 2018 07:54 PM, Ville Syrjälä wrote:
> > On Thu, Mar 29, 2018 at 07:39:07PM +0530, Ramalingam C wrote:
> >> HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk
> >> is enabled. Using the I915 power well infrastruture, above requirement
> >> is verified.
> >>
> >> This patch enables the hdcp initialization for HSW, BDW, and BXT.
> >>
> >> v2:
> >>    Choose the PW# based on the platform.
> >>
> >> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
> >> Reviewed-by: Sean Paul <seanpaul at chromium.org>
> >> ---
> >>   drivers/gpu/drm/i915/intel_hdcp.c | 41 +++++++++++++++++++++++++++++++++++++--
> >>   1 file changed, 39 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c
> >> index f77d956b2b18..d8af09b46a44 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> >> @@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
> >>   	return 0;
> >>   }
> >>   
> >> +static bool hdcp_key_loadable(struct drm_i915_private *dev_priv)
> >> +{
> >> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> >> +	struct i915_power_well *power_well;
> >> +	enum i915_power_well_id id;
> >> +	bool enabled = false;
> >> +
> >> +	/*
> >> +	 * On HSW and BDW, Display HW loads the Key as soon as Display resumes.
> >> +	 * On all BXT+, SW can load the keys only when the PW#1 is turned on.
> >> +	 */
> >> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> >> +		id = HSW_DISP_PW_GLOBAL;
> >> +	else
> >> +		id = SKL_DISP_PW_1;
> >> +
> >> +	mutex_lock(&power_domains->lock);
> >> +
> >> +	/* PG1 (power well #1) needs to be enabled */
> >> +	for_each_power_well(dev_priv, power_well) {
> >> +		if (power_well->id == id) {
> >> +			enabled = power_well->ops->is_enabled(dev_priv,
> >> +							      power_well);
> >> +			break;
> >> +		}
> >> +	}
> >> +	mutex_unlock(&power_domains->lock);
> >> +
> >> +	/*
> >> +	 * Another req for hdcp key loadability is enabled state of pll for
> >> +	 * cdclk. Without active crtc we wont land here. So we are assuming that
> >> +	 * cdclk is already on.
> >> +	 */
> >> +
> >> +	return enabled;
> >> +}
> >> +
> >>   static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv)
> >>   {
> >>   	I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER);
> >> @@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector *connector)
> >>   	DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n",
> >>   		      connector->base.name, connector->base.base.id);
> >>   
> >> -	if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) {
> >> -		DRM_ERROR("PG1 is disabled, cannot load keys\n");
> >> +	if (!hdcp_key_loadable(dev_priv)) {
> >> +		DRM_ERROR("HDCP key Load is not possible\n");
> >>   		return -ENXIO;
> >>   	}
> > If you need the power well then why aren't you grabbing the correct
> > power domain reference somewhere?
> 
> Ville,
> 
> As HDCP is supposed to be enabled after the basic display is up, power well #1 is supposed to be enabled.
> If not that is mostly an error w.r.t HDCP.
> 
> So at this point we just want to verify whether required PW is up and HDCP key can be loaded from the HW. Else fail the HDCP request.

So this is in practice dead code to deal with a hypothetical bug
elsewhere in the driver?

-- 
Ville Syrjälä
Intel OTC