| Message ID | 1483365281-10569-1-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira wrote: > After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization to > runtime init"), scalers are not initialized properly for skl and glk > since num_scalers is left as 0 for those platforms. Next question is why this user visible change is not causing test failures in BAT? -Chris
I am wondering why the number of sprite initialization is not done in runtime init for skylake/glk similarly. On 1/2/2017 8:27 PM, Chris Wilson wrote: > On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira wrote: >> After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization to >> runtime init"), scalers are not initialized properly for skl and glk >> since num_scalers is left as 0 for those platforms. > > Next question is why this user visible change is not causing test > failures in BAT? > -Chris >
On Mon, 2017-01-02 at 14:57 +0000, Chris Wilson wrote: > On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira wrote: > > > > After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization > > to > > runtime init"), scalers are not initialized properly for skl and glk > > since num_scalers is left as 0 for those platforms. > Next question is why this user visible change is not causing test > failures in BAT? There isn't a single test in BAT that tries to setup a plane with scaling. The kms_panel_fitting and kms_plane_scaling tests would have caught the issue, but they are not part of BAT. kms_plane and kms_universal_plane are also not part of BAT, and they also don't test scaling. Ander
On Mon, 2017-01-02 at 21:19 +0530, Maiti, Nabendu Bikash wrote: > I am wondering why the number of sprite initialization is not done in > runtime init for skylake/glk similarly. It is done in the "else if (INTEL_GEN(dev_priv) >= 5)" branch. The comment explains why the topmost plane is not enabled. Ander > > On 1/2/2017 8:27 PM, Chris Wilson wrote: > > > > On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira wrote: > > > > > > After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers > > > initialization to > > > runtime init"), scalers are not initialized properly for skl and glk > > > since num_scalers is left as 0 for those platforms. > > Next question is why this user visible change is not causing test > > failures in BAT? > > -Chris > >
On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira wrote: > After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization to > runtime init"), scalers are not initialized properly for skl and glk > since num_scalers is left as 0 for those platforms. > > Fixes: 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization to runtime init") > Cc: Nabendu Maiti <nabendu.bikash.maiti@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> (v2) > Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: intel-gfx@lists.freedesktop.org > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Fixes my CCS test. What happened was apparently the BIOS leaving the scaler enabled and then when i915 took over all but the preferred mode of the display came out garbled on account of the scaler output not fitting within the h/vactive of the mode. Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_device_info.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index 1b5ffc4..f642f6d 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -310,6 +310,12 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) > struct intel_device_info *info = mkwrite_device_info(dev_priv); > enum pipe pipe; > > + if (INTEL_GEN(dev_priv) >= 9) { > + info->num_scalers[PIPE_A] = 2; > + info->num_scalers[PIPE_B] = 2; > + info->num_scalers[PIPE_C] = 1; > + } > + > /* > * Skylake and Broxton currently don't expose the topmost plane as its > * use is exclusive with the legacy cursor and we only want to expose > @@ -325,9 +331,6 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) > info->num_sprites[PIPE_A] = 2; > info->num_sprites[PIPE_B] = 2; > info->num_sprites[PIPE_C] = 1; > - info->num_scalers[PIPE_A] = 2; > - info->num_scalers[PIPE_B] = 2; > - info->num_scalers[PIPE_C] = 1; > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > for_each_pipe(dev_priv, pipe) > info->num_sprites[pipe] = 2; > -- > 2.5.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2017-01-03 at 15:35 +0200, Ville Syrjälä wrote: > On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira wrote: > > > > After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization > > to > > runtime init"), scalers are not initialized properly for skl and glk > > since num_scalers is left as 0 for those platforms. > > > > Fixes: 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization to > > runtime init") > > Cc: Nabendu Maiti <nabendu.bikash.maiti@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> (v2) > > Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> > > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@inte > > l.com> > Fixes my CCS test. What happened was apparently the BIOS leaving the > scaler enabled and then when i915 took over all but the preferred mode > of the display came out garbled on account of the scaler output not > fitting within the h/vactive of the mode. > > Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Pushed. Thanks for reviewing. Ander > > > > > --- > > drivers/gpu/drm/i915/intel_device_info.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c > > b/drivers/gpu/drm/i915/intel_device_info.c > > index 1b5ffc4..f642f6d 100644 > > --- a/drivers/gpu/drm/i915/intel_device_info.c > > +++ b/drivers/gpu/drm/i915/intel_device_info.c > > @@ -310,6 +310,12 @@ void intel_device_info_runtime_init(struct > > drm_i915_private *dev_priv) > > struct intel_device_info *info = mkwrite_device_info(dev_priv); > > enum pipe pipe; > > > > + if (INTEL_GEN(dev_priv) >= 9) { > > + info->num_scalers[PIPE_A] = 2; > > + info->num_scalers[PIPE_B] = 2; > > + info->num_scalers[PIPE_C] = 1; > > + } > > + > > /* > > * Skylake and Broxton currently don't expose the topmost plane as > > its > > * use is exclusive with the legacy cursor and we only want to > > expose > > @@ -325,9 +331,6 @@ void intel_device_info_runtime_init(struct > > drm_i915_private *dev_priv) > > info->num_sprites[PIPE_A] = 2; > > info->num_sprites[PIPE_B] = 2; > > info->num_sprites[PIPE_C] = 1; > > - info->num_scalers[PIPE_A] = 2; > > - info->num_scalers[PIPE_B] = 2; > > - info->num_scalers[PIPE_C] = 1; > > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > > for_each_pipe(dev_priv, pipe) > > info->num_sprites[pipe] = 2; > > -- > > 2.5.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
On Tue, 2017-01-03 at 12:26 +0200, Ander Conselvan De Oliveira wrote: > On Mon, 2017-01-02 at 14:57 +0000, Chris Wilson wrote: > > > > On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira wrote: > > > > > > > > > After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers > > > initialization > > > to > > > runtime init"), scalers are not initialized properly for skl and glk > > > since num_scalers is left as 0 for those platforms. > > Next question is why this user visible change is not causing test > > failures in BAT? > There isn't a single test in BAT that tries to setup a plane with scaling. The > kms_panel_fitting and kms_plane_scaling tests would have caught the issue, but > they are not part of BAT. kms_plane and kms_universal_plane are also not part > of > BAT, and they also don't test scaling. Is it feasible to add one of those tests to BAT? Would it help with the time constraints if we run it only on gen9+? There's already a test for gen9 specific things in kms_universal_plane, maybe we could use that? Ander
On Tue, Jan 03, 2017 at 03:49:15PM +0200, Ander Conselvan De Oliveira wrote: > On Tue, 2017-01-03 at 12:26 +0200, Ander Conselvan De Oliveira wrote: > > On Mon, 2017-01-02 at 14:57 +0000, Chris Wilson wrote: > > > > > > On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira wrote: > > > > > > > > > > > > After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers > > > > initialization > > > > to > > > > runtime init"), scalers are not initialized properly for skl and glk > > > > since num_scalers is left as 0 for those platforms. > > > Next question is why this user visible change is not causing test > > > failures in BAT? > > There isn't a single test in BAT that tries to setup a plane with scaling. The > > kms_panel_fitting and kms_plane_scaling tests would have caught the issue, but > > they are not part of BAT. kms_plane and kms_universal_plane are also not part > > of > > BAT, and they also don't test scaling. > > Is it feasible to add one of those tests to BAT? Would it help with the time > constraints if we run it only on gen9+? There's already a test for gen9 specific > things in kms_universal_plane, maybe we could use that? Yes, it is definitely a hole that needs addressing, and needs to be checked on all gen (some just have more versatile panel fitters than others). The challenge for you is to devise the smallest test with the widest coverage of the scalers - which may just be an existing test. -Chris
HI, > -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of > Ander Conselvan De Oliveira > Sent: Tuesday, January 3, 2017 3:49 PM > To: Chris Wilson <chris@chris-wilson.co.uk>; Sarvela, Tomi P > <tomi.p.sarvela@intel.com>; Latvala, Petri <petri.latvala@intel.com> > Cc: Vetter, Daniel <daniel.vetter@intel.com>; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Initialize num_scalers for skl and glk > too > > On Tue, 2017-01-03 at 12:26 +0200, Ander Conselvan De Oliveira wrote: > > On Mon, 2017-01-02 at 14:57 +0000, Chris Wilson wrote: > > > > > > On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira > wrote: > > > > > > > > > > > > After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers > > > > initialization to runtime init"), scalers are not initialized > > > > properly for skl and glk since num_scalers is left as 0 for those > > > > platforms. > > > Next question is why this user visible change is not causing test > > > failures in BAT? > > There isn't a single test in BAT that tries to setup a plane with > > scaling. The kms_panel_fitting and kms_plane_scaling tests would have > > caught the issue, but they are not part of BAT. kms_plane and > > kms_universal_plane are also not part of BAT, and they also don't test > > scaling. > > Is it feasible to add one of those tests to BAT? Would it help with the time > constraints if we run it only on gen9+? There's already a test for gen9 specific > things in kms_universal_plane, maybe we could use that? We should test drive first how reliable test is and if reliable (not flip-flopping) yes. Petri, proposals? Jani Saarinen Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 1b5ffc4..f642f6d 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -310,6 +310,12 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) struct intel_device_info *info = mkwrite_device_info(dev_priv); enum pipe pipe; + if (INTEL_GEN(dev_priv) >= 9) { + info->num_scalers[PIPE_A] = 2; + info->num_scalers[PIPE_B] = 2; + info->num_scalers[PIPE_C] = 1; + } + /* * Skylake and Broxton currently don't expose the topmost plane as its * use is exclusive with the legacy cursor and we only want to expose @@ -325,9 +331,6 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) info->num_sprites[PIPE_A] = 2; info->num_sprites[PIPE_B] = 2; info->num_sprites[PIPE_C] = 1; - info->num_scalers[PIPE_A] = 2; - info->num_scalers[PIPE_B] = 2; - info->num_scalers[PIPE_C] = 1; } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { for_each_pipe(dev_priv, pipe) info->num_sprites[pipe] = 2;
After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization to runtime init"), scalers are not initialized properly for skl and glk since num_scalers is left as 0 for those platforms. Fixes: 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization to runtime init") Cc: Nabendu Maiti <nabendu.bikash.maiti@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> (v2) Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- drivers/gpu/drm/i915/intel_device_info.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)