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 <ramalinga...@intel.com>
> > >> Reviewed-by: Sean Paul <seanp...@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?

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to