Re: [PATCH 1/2] drm/i915: Runtime disable for eDP DRRS

C, Ramalingam
2017-11-01T16:44:27Z

> -----Original Message-----
> From: Vivi, Rodrigo
> Sent: Wednesday, November 1, 2017 12:27 AM
> To: C, Ramalingam <ramalingam.c@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Zanoni, Paulo R
> <paulo.r.zanoni@intel.com>; chris@chris-wilson.co.uk
> Subject: Re: [PATCH 1/2] drm/i915: Runtime disable for eDP DRRS
>
> On Tue, Oct 31, 2017 at 09:20:42AM +0000, Ramalingam C wrote:
> > From: "C, Ramalingam" <ramalingam.c@intel.com>
> >
> > Module parameter enable_drrs(Boolean flag) is added to control the
> > eDP Idleness drrs enable flow.
>
> This goes on the opposite direction of the current trends.
>
> Well, I'm a big fan of the parameters, but there's a big effort
> going on to remove all kernel parameters. I believe it will
> be just a matter of time that we have to remove fbc and psr as well.
> So probably not a good idea to add something now that we will
> have to rework soon.
>
> Maybe we could add a on/off toggle on DRRS now and then
> when we remove the parameter for fbc and psr we also add toggles
> on debugfs...

Based on our offline discussion, I will use the debugfs to toggle the DRRS instead of modparams.
Once the code is ready I will share it for the review.

Thanks
--Ram
>
> Thanks,
> Rodrigo.
>
> >
> > Modification to this module parameter will be considered on next
> > eDP_DRRS enable flow. So after module parameter update, a modeset
> > will help to modify the feature state as per the module parameter's
> > current state.
> >
> > Possibility of disabling the DRRS, enables the testing of the
> > frontbuffer tracking based features (FBC, DRRS and PSR) as standalone
> > or any combination of the set.
> >
> > Signed-off-by: C, Ramalingam <ramalingam.c@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_params.c | 3 +++
> > drivers/gpu/drm/i915/i915_params.h | 3 ++-
> > drivers/gpu/drm/i915/intel_dp.c | 6 ++++++
> > 3 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> > index b4faeb6aa2bd..32f06bb74f9d 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -190,3 +190,6 @@ i915_param_named(enable_dpcd_backlight, bool,
> 0600,
> >
> > i915_param_named(enable_gvt, bool, 0400,
> > "Enable support for Intel GVT-g graphics virtualization host
> support(default:false)");
> > +
> > +i915_param_named_unsafe(enable_drrs, bool, 0600,
> > + "Enable DRRS. (True=Enabled, False=Disabled [Default])");
> > diff --git a/drivers/gpu/drm/i915/i915_params.h
> b/drivers/gpu/drm/i915/i915_params.h
> > index c7292268ed43..3c6fdce1c122 100644
> > --- a/drivers/gpu/drm/i915/i915_params.h
> > +++ b/drivers/gpu/drm/i915/i915_params.h
> > @@ -67,7 +67,8 @@
> > param(bool, nuclear_pageflip, false) \
> > param(bool, enable_dp_mst, true) \
> > param(bool, enable_dpcd_backlight, false) \
> > - param(bool, enable_gvt, false)
> > + param(bool, enable_gvt, false) \
> > + param(bool, enable_drrs, false)
> >
> > #define MEMBER(T, member, ...) T member;
> > struct i915_params {
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index ca48bce23a6f..ff9964cf15cd 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5568,6 +5568,11 @@ void intel_edp_drrs_enable(struct intel_dp
> *intel_dp,
> > return;
> > }
> >
> > + if (!i915_modparams.enable_drrs) {
> > + DRM_DEBUG_KMS("DRRS is disabled from modparams\n");
> > + return;
> > + }
> > +
> > mutex_lock(&dev_priv->drrs.mutex);
> > if (WARN_ON(dev_priv->drrs.dp)) {
> > DRM_ERROR("DRRS already enabled\n");
> > @@ -5817,6 +5822,7 @@ intel_dp_drrs_init(struct intel_connector
> *intel_connector,
> > }
> >
> > dev_priv->drrs.type = dev_priv->vbt.drrs_type;
> > + i915_modparams.enable_drrs = true;
> >
> > dev_priv->drrs.refresh_rate_type = DRRS_HIGH_RR;
> > DRM_DEBUG_KMS("seamless DRRS supported for eDP panel.\n");
> > --
> > 2.7.4
> >