Re: [PATCH 19/26] drm/i915: move adjusted_mode checks from fbc_update to fbc_enable

Zanoni, Paulo R
2015-10-29T17:58:15Z

Em Qui, 2015-10-29 às 13:59 +0100, Maarten Lankhorst escreveu:
> Op 27-10-15 om 17:50 schreef Paulo Zanoni:
> > These things can't change without a full modeset.
> False! Fastset can update parameters too. Although I don't think it
> currently prevents DBLSCAN updates,
> so maybe make sure cfb enable/disable is called when updating pipe
> too?

You mean that if we change for an interlaced (or double-scanned) to a
non-interlaced (or non-double-scanned) mode we won't get
crtc_disable+crtc_enable? That's surprising. I'll take a better look
here and try to write a test case. Is this just through some specific
API (like drmModeSetPlane)?

I'm trying to get test cases for every single bug I discover or
introduce.

> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_fbc.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > b/drivers/gpu/drm/i915/intel_fbc.c
> > index 0ba25b9..6aa9af8 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -825,7 +825,6 @@ static void __intel_fbc_update(struct
> > intel_crtc *crtc)
> >   struct drm_i915_private *dev_priv = crtc->base.dev-
> > >dev_private;
> >   struct drm_framebuffer *fb;
> >   struct drm_i915_gem_object *obj;
> > - const struct drm_display_mode *adjusted_mode;
> >
> >   WARN_ON(!mutex_is_locked(&dev_priv->fbc.lock));
> >
> > @@ -844,13 +843,6 @@ static void __intel_fbc_update(struct
> > intel_crtc *crtc)
> >
> >   fb = crtc->base.primary->fb;
> >   obj = intel_fb_obj(fb);
> > - adjusted_mode = &crtc->config->base.adjusted_mode;
> > -
> > - if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
> > -     (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
> > - set_no_fbc_reason(dev_priv, "incompatible mode");
> > - goto out_disable;
> > - }
> >
> >   if (!intel_fbc_hw_tracking_covers_screen(crtc)) {
> >   set_no_fbc_reason(dev_priv, "mode too large for
> > compression");
> > @@ -1076,6 +1068,8 @@ void intel_fbc_flip_prepare(struct
> > drm_i915_private *dev_priv,
> >  void intel_fbc_enable(struct intel_crtc *crtc)
> >  {
> >   struct drm_i915_private *dev_priv = crtc->base.dev-
> > >dev_private;
> > + const struct drm_display_mode *adjusted_mode =
> > + &crtc->config-
> > >base.adjusted_mode;
> >
> >   if (!fbc_supported(dev_priv))
> >   return;
> > @@ -1110,6 +1104,12 @@ void intel_fbc_enable(struct intel_crtc
> > *crtc)
> >   goto out;
> >   }
> >
> > + if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
> > +     (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
> > + set_no_fbc_reason(dev_priv, "incompatible mode");
> > + goto out;
> > + }
> > +
> >   if (intel_fbc_alloc_cfb(crtc)) {
> >   set_no_fbc_reason(dev_priv, "not enough stolen
> > memory");
> >   goto out;
>