On Fri, Apr 06, 2018 at 07:02:02PM +0300, Ville Syrjälä wrote: > 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@xxxxxxxxx> > > >> Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx> > > >> --- > > >> 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? Yeah looks like it should be wrapped in a WARN_ON, or maybe outright thrown out. The unclaimed mmio debug stuff will catch when this happens (or well, should). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel