Re: [PATCH 14/21] drm/i915: use current scaler state during readout_hw_state.

From: Daniel Vetter <daniel@ffwll.ch>
To: "Konduru, Chandra" <chandra.konduru@intel.com>
Cc: "Conselvan De Oliveira,
	Ander" <ander.conselvan.de.oliveira@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Vetter, Daniel" <daniel.vetter@intel.com>
Subject: Re: [PATCH 14/21] drm/i915: use current scaler state during readout_hw_state.
Date: Wed, 18 Mar 2015 09:06:08 +0100	[thread overview]
Message-ID: <20150318080608.GR21993@phenom.ffwll.local> (raw)
In-Reply-To: <76A9B330A4D78C4D99CB292C4CC06C0E36F7A0B3@fmsmsx101.amr.corp.intel.com>

On Tue, Mar 17, 2015 at 06:54:30PM +0000, Konduru, Chandra wrote:
> 
> 
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> > Sent: Tuesday, March 17, 2015 7:20 AM
> > To: Konduru, Chandra
> > Cc: intel-gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander; Vetter, Daniel
> > Subject: Re: [Intel-gfx] [PATCH 14/21] drm/i915: use current scaler state during
> > readout_hw_state.
> > 
> > On Sat, Mar 14, 2015 at 10:55:39PM -0700, Chandra Konduru wrote:
> > > During readout_hw_state, whole pipe_config is built reading hw.
> > > But rebuilding scaler state from hw requires,
> > >  - reading all planes and find its corresponding index in order to set
> > >    its bits in scaler_users
> > >  - reading cdclk and adjusted mode crtc clk in order to regenerate
> > >    min scaling ratios.
> > >  - some values directly from bspec
> > >
> > > To simplify things, for now using sw scaler state as readout state.
> > >
> > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > 
> > At least the crtc scaler might get used by the bios for boot-up with non-native
> > resolution. I do think we need to properly track those parts (and also double-
> > check the hw state in pipe config compare function like we do for the old pfit
> > state).
> 
> To cover above case, will populate the crtc scaler state.
> 
> But population of scalers used for planes is tricky, because 
> in general there is no population of planes or its states from hw.
> From scaler point of this may be ok because there aren't any 
> scalers in use for planes.

Yeah plane scaler state is completely different. Imo it's ok to not care
about that - as you spotted we almost completely lack state readout
support for planes. No need to fix that in your series ;-)

> > Maybe we need to always compute limits directly instead of storing the in the
> > state structures?
> 
> We are adjusting/computing some of limits from hw but not all
> limits. 
> Along with crtc_scaler population, will add code to populate
> limits that can be computed from hw. Rest will carry from previous
> state.

My suggestion was to completely remove the limit fields from the scaler
state and always recompute them when needed. I think that would simplify
the code in a few other places, and reduce changes that these computed
values would get out of sync.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx