diff mbox

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

Message ID 1426398946-5900-15-git-send-email-chandra.konduru@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chandra Konduru March 15, 2015, 5:55 a.m. UTC
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>
---
 drivers/gpu/drm/i915/intel_display.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Daniel Vetter March 17, 2015, 2:19 p.m. UTC | #1
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).

Maybe we need to always compute limits directly instead of storing the in
the state structures?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9c3c6b1..b1d7036 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14292,7 +14292,10 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  	int i;
>  
>  	for_each_intel_crtc(dev, crtc) {
> +		struct intel_crtc_scaler_state scaler_state;
> +		scaler_state = crtc->config->scaler_state;
>  		memset(crtc->config, 0, sizeof(*crtc->config));
> +		crtc->config->scaler_state = scaler_state;
>  
>  		crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Chandra Konduru March 17, 2015, 6:54 p.m. UTC | #2
> -----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.

> 
> 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.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |    3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 9c3c6b1..b1d7036 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14292,7 +14292,10 @@ static void
> intel_modeset_readout_hw_state(struct drm_device *dev)
> >  	int i;
> >
> >  	for_each_intel_crtc(dev, crtc) {
> > +		struct intel_crtc_scaler_state scaler_state;
> > +		scaler_state = crtc->config->scaler_state;
> >  		memset(crtc->config, 0, sizeof(*crtc->config));
> > +		crtc->config->scaler_state = scaler_state;
> >
> >  		crtc->config->quirks |=
> PIPE_CONFIG_QUIRK_INHERITED_MODE;
> >
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Daniel Vetter March 18, 2015, 8:06 a.m. UTC | #3
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
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9c3c6b1..b1d7036 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14292,7 +14292,10 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	int i;
 
 	for_each_intel_crtc(dev, crtc) {
+		struct intel_crtc_scaler_state scaler_state;
+		scaler_state = crtc->config->scaler_state;
 		memset(crtc->config, 0, sizeof(*crtc->config));
+		crtc->config->scaler_state = scaler_state;
 
 		crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;