| Message ID | 1418054960-1403-4-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > .. because it would be a waste of time, so move the place where the > check is done. Also, with this we won't risk printing "more than one > pipe active, disabling compression" or "no output, disabling" when FBC > is actually disabled. > > This patch also represents a small behavior difference when using > i915.powersave=0: it is now exactly the same as i915.enable_fbc=0 on > this part of the code. I always ask myself if we should continue using this i915.powersave here. I vaguely remember someone complaining when I tried to re-org this i915.powersave making it a umbrealla for all powersaving features and the complain was to avoid more than one variable for feature... to avoid superset of parameters, or something like that. Or if we keep this here shouldn't we also put this to disable psr? Anyway this can be another patch. Daniel, please let me know what you want with this parameters that I can change that later. So for this patch: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_fbc.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c > index 7686573..4647d57 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -521,10 +521,16 @@ void intel_fbc_update(struct drm_device *dev) > return; > } > > - if (!i915.powersave) { > + if (i915.enable_fbc < 0) { > + if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) > + DRM_DEBUG_KMS("disabled per chip default\n"); > + goto out_disable; > + } > + > + if (!i915.enable_fbc || !i915.powersave) { > if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) > DRM_DEBUG_KMS("fbc disabled per module param\n"); > - return; > + goto out_disable; > } > > /* > @@ -559,16 +565,6 @@ void intel_fbc_update(struct drm_device *dev) > obj = intel_fb_obj(fb); > adjusted_mode = &intel_crtc->config.adjusted_mode; > > - if (i915.enable_fbc < 0) { > - if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) > - DRM_DEBUG_KMS("disabled per chip default\n"); > - goto out_disable; > - } > - if (!i915.enable_fbc) { > - if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) > - DRM_DEBUG_KMS("fbc disabled per module param\n"); > - goto out_disable; > - } > if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) || > (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) { > if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE)) > -- > 2.1.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Dec 12, 2014 at 04:53:45PM -0800, Rodrigo Vivi wrote: > On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote: > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > .. because it would be a waste of time, so move the place where the > > check is done. Also, with this we won't risk printing "more than one > > pipe active, disabling compression" or "no output, disabling" when FBC > > is actually disabled. > > > > This patch also represents a small behavior difference when using > > i915.powersave=0: it is now exactly the same as i915.enable_fbc=0 on > > this part of the code. > > I always ask myself if we should continue using this i915.powersave here. > I vaguely remember someone complaining when I tried to re-org this > i915.powersave making it a umbrealla for all powersaving features and > the complain was to avoid more than one variable for feature... to > avoid superset of parameters, or something like that. > > Or if we keep this here shouldn't we also put this to disable psr? > > Anyway this can be another patch. Daniel, please let me know what you > want with this parameters that I can change that later. Imo powersave has outlived its usefulness and it can die. We've long ago started having specific options for this. So maybe create a patch to mark it as a debug option and then 1 kernel later kill it? -Daneil
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 7686573..4647d57 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -521,10 +521,16 @@ void intel_fbc_update(struct drm_device *dev) return; } - if (!i915.powersave) { + if (i915.enable_fbc < 0) { + if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) + DRM_DEBUG_KMS("disabled per chip default\n"); + goto out_disable; + } + + if (!i915.enable_fbc || !i915.powersave) { if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) DRM_DEBUG_KMS("fbc disabled per module param\n"); - return; + goto out_disable; } /* @@ -559,16 +565,6 @@ void intel_fbc_update(struct drm_device *dev) obj = intel_fb_obj(fb); adjusted_mode = &intel_crtc->config.adjusted_mode; - if (i915.enable_fbc < 0) { - if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) - DRM_DEBUG_KMS("disabled per chip default\n"); - goto out_disable; - } - if (!i915.enable_fbc) { - if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) - DRM_DEBUG_KMS("fbc disabled per module param\n"); - goto out_disable; - } if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) || (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) { if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))