Message ID | 1426398946-5900-15-git-send-email-chandra.konduru@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
> -----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
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 --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;
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(+)