Re: [PATCH 04/18] drm/i915: extract crtc_is_valid() on the FBC code

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 22, 2015 at 07:26:36PM +0000, Zanoni, Paulo R wrote:
> Em Qui, 2015-10-22 às 09:52 +0200, Maarten Lankhorst escreveu:
> > Op 20-10-15 om 15:49 schreef Paulo Zanoni:
> > > We're going to kill intel_fbc_find_crtc(), that's why a big part of
> > > the logic moved from intel_fbc_find_crtc() to crtc_is_valid().
> > > 
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_fbc.c | 26 ++++++++++++++++----------
> > >  1 file changed, 16 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > > b/drivers/gpu/drm/i915/intel_fbc.c
> > > index b9cfd16..1162787 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > @@ -538,27 +538,33 @@ static void set_no_fbc_reason(struct
> > > drm_i915_private *dev_priv,
> > >  	DRM_DEBUG_KMS("Disabling FBC: %s\n", reason);
> > >  }
> > >  
> > > +static bool crtc_is_valid(struct intel_crtc *crtc)
> > > +{
> > > +	struct drm_i915_private *dev_priv = crtc->base.dev-
> > > >dev_private;
> > > +	enum pipe pipe = crtc->pipe;
> > > +
> > > +	if ((IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >=
> > > 8) &&
> > > +	    pipe != PIPE_A)
> > > +		return false;
> > > +
> > > +	return intel_crtc_active(&crtc->base) &&
> > > +	       to_intel_plane_state(crtc->base.primary->state)-
> > > >visible &&
> > > +	       crtc->base.primary->fb != NULL;
> > > +}
> > > 
> > I've been considering something like this, but could it be changed to
> > take atomic states as arguments?
> 
> I'm not sure. All I know is that this is still (both now and at the end
> of the series) being called while the CRTC is already enabled, not
> during modeset paths. The visible/invisible case seems to be the most
> important check. The other two checks are like this simply because this
> code was written a loooong time ago and was never updated while the
> rest of the tree evolved.

You know, fbc_score could be correctly calculated when we are
calculating the rest of the primary plane state ;)

> 
> Since there are so many things to update on the FBC code I was just
> trying to move the code around without really over-analyzing it since
> it seemed to be working. But I guess these days any line of i915 code
> touched on a patch has to be perfect :)
> 
> Anyway, I'll try to update this code on this series since you & Chris &
> Ville already asked about it.
> 
> > That way it will be easier to use when >1 flip depth is allowed in
> > the future, and intel_crtc_active is not
> > a check that should be used here.
> > 
> > At some point in the near future I want to convert intel_unpin_work
> > to take the previous and next crtc/plane
> > states, that would become a lot easier if this code would be more
> > atomic like. :)
> > 
> > ~Maarten
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx