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

From: "Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
To: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"maarten.lankhorst@linux.intel.com"
	<maarten.lankhorst@linux.intel.com>
Subject: Re: [PATCH 19/26] drm/i915: move adjusted_mode checks from fbc_update to fbc_enable
Date: Thu, 29 Oct 2015 17:58:14 +0000	[thread overview]
Message-ID: <1446141494.2750.56.camel@intel.com> (raw)
In-Reply-To: <5632184F.7040203@linux.intel.com>

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;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx