All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
@ 2017-07-19 22:50 Imre Deak
  2017-07-19 22:50 ` [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout Imre Deak
                   ` (6 more replies)
  0 siblings, 7 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-19 22:50 UTC (permalink / raw)
  To: intel-gfx
  Cc: Shashank Sharma, Ville Syrjälä, Chandra Konduru,
	Matt Roper, stable

The scaler allocation code depends on a non-zero default value for the
crtc scaler_id, so make sure we initialize the scaler state accordingly
even if the crtc is off. This fixes at least an initial YUV420 modeset
(added in a follow-up patchset by Shashank) when booting with the screen
off: after the initial HW readout and modeset which enables the scaler a
subsequent modeset will disable the scaler which isn't properly
allocated. This results in a funky HW state where the pipe scaler HW
registers can't be modified and the normally black screen is grey and
shifted to the right or jitters.

The problem was revealed by Shashank's YUV420 patchset and first
reported by Ville.

Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chandra Konduru <chandra.konduru@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: <stable@vger.kernel.org> # 4.11.x
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
Signed-off-by: Imre Deak <imre.deak@intel.com>

---

[ Older stable versions need backporting, so that's for a follow-up ]
---
 drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7774f3465fbc..8a38e64b1931 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	u64 power_domain_mask;
 	bool active;
 
+	if (INTEL_GEN(dev_priv) >= 9) {
+		intel_crtc_init_scalers(crtc, pipe_config);
+
+		pipe_config->scaler_state.scaler_id = -1;
+		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
+	}
+
 	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
@@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	pipe_config->gamma_mode =
 		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
 
-	if (INTEL_GEN(dev_priv) >= 9) {
-		intel_crtc_init_scalers(crtc, pipe_config);
-
-		pipe_config->scaler_state.scaler_id = -1;
-		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
-	}
-
 	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
 	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
 		power_domain_mask |= BIT_ULL(power_domain);
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout
  2017-07-19 22:50 [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Imre Deak
@ 2017-07-19 22:50 ` Imre Deak
  2017-07-21 13:14   ` Mahesh Kumar
  2017-07-19 23:09 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Fix scaler init during CRTC HW state readout Patchwork
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Imre Deak @ 2017-07-19 22:50 UTC (permalink / raw)
  To: intel-gfx

The crtc state starts out being bzero'd, so no need to clear
scaler_users. Also intel_crtc_init_scalers() knows already which
platforms have scalers, so no need for the platform check here.
Similarly intel_crtc_init_scalers() will init scaler_id as required,
so no need to do it here separately.

Cc: Chandra Konduru <chandra.konduru@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8a38e64b1931..7210f418e9c0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9132,12 +9132,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	u64 power_domain_mask;
 	bool active;
 
-	if (INTEL_GEN(dev_priv) >= 9) {
-		intel_crtc_init_scalers(crtc, pipe_config);
-
-		pipe_config->scaler_state.scaler_id = -1;
-		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
-	}
+	intel_crtc_init_scalers(crtc, pipe_config);
 
 	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-19 22:50 [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Imre Deak
  2017-07-19 22:50 ` [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout Imre Deak
@ 2017-07-19 23:09 ` Patchwork
  2017-07-20  8:58   ` [Intel-gfx] " Jani Nikula
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2017-07-19 23:09 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Fix scaler init during CRTC HW state readout
URL   : https://patchwork.freedesktop.org/series/27607/
State : success

== Summary ==

Series 27607v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/27607/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                warn       -> FAIL       (fi-snb-2600) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                warn       -> SKIP       (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                warn       -> DMESG-FAIL (fi-pnv-d510) fdo#101597 +1

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:11  time:454s
fi-bdw-gvtdvm    total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:14  time:439s
fi-blb-e6850     total:279  pass:0    dwarn:0   dfail:1   fail:0   skip:54  time:359s
fi-bsw-n3050     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:36  time:539s
fi-bxt-j4205     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:19  time:518s
fi-byt-j1900     total:279  pass:0    dwarn:0   dfail:1   fail:0   skip:24  time:490s
fi-byt-n2820     total:279  pass:0    dwarn:0   dfail:1   fail:0   skip:28  time:493s
fi-glk-2a        total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:19  time:599s
fi-hsw-4770      total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:16  time:435s
fi-hsw-4770r     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:16  time:416s
fi-ilk-650       total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:50  time:414s
fi-ivb-3520m     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:18  time:504s
fi-ivb-3770      total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:18  time:473s
fi-kbl-7500u     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:18  time:467s
fi-kbl-7560u     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:10  time:575s
fi-kbl-r         total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:18  time:581s
fi-pnv-d510      total:279  pass:0    dwarn:0   dfail:3   fail:0   skip:55  time:559s
fi-skl-6260u     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:10  time:467s
fi-skl-6700hq    total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:17  time:594s
fi-skl-6700k     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:18  time:467s
fi-skl-6770hq    total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:10  time:473s
fi-skl-gvtdvm    total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:13  time:436s
fi-skl-x1585l    total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:11  time:470s
fi-snb-2520m     total:279  pass:0    dwarn:0   dfail:0   fail:0   skip:28  time:545s
fi-snb-2600      total:279  pass:0    dwarn:0   dfail:0   fail:1   skip:29  time:404s

f7e80ae2a77f233ffca8fcf5739b26e2bd9a80a6 drm-tip: 2017y-07m-19d-18h-16m-25s UTC integration manifest
e51c5fc drm/i915: Simplify scaler init during CRTC HW readout
c296387 drm/i915: Fix scaler init during CRTC HW state readout

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5239/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-19 22:50 [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Imre Deak
@ 2017-07-20  8:58   ` Jani Nikula
  2017-07-19 23:09 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Fix scaler init during CRTC HW state readout Patchwork
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2017-07-20  8:58 UTC (permalink / raw)
  To: Imre Deak, intel-gfx; +Cc: stable

On Thu, 20 Jul 2017, Imre Deak <imre.deak@intel.com> wrote:
> The scaler allocation code depends on a non-zero default value for the
> crtc scaler_id, so make sure we initialize the scaler state accordingly
> even if the crtc is off. This fixes at least an initial YUV420 modeset
> (added in a follow-up patchset by Shashank) when booting with the screen
> off: after the initial HW readout and modeset which enables the scaler a
> subsequent modeset will disable the scaler which isn't properly
> allocated. This results in a funky HW state where the pipe scaler HW
> registers can't be modified and the normally black screen is grey and
> shifted to the right or jitters.
>
> The problem was revealed by Shashank's YUV420 patchset and first
> reported by Ville.
>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chandra Konduru <chandra.konduru@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: <stable@vger.kernel.org> # 4.11.x
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
> Signed-off-by: Imre Deak <imre.deak@intel.com>
>
> ---
>
> [ Older stable versions need backporting, so that's for a follow-up ]

I thought we'd annotate cc: stable with all the kernels that need the
fix, not according to where the fix applies as-is. In this case, it
would be v4.2+, right?

BR,
Jani.



> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7774f3465fbc..8a38e64b1931 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	u64 power_domain_mask;
>  	bool active;
>  
> +	if (INTEL_GEN(dev_priv) >= 9) {
> +		intel_crtc_init_scalers(crtc, pipe_config);
> +
> +		pipe_config->scaler_state.scaler_id = -1;
> +		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> +	}
> +
>  	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>  		return false;
> @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	pipe_config->gamma_mode =
>  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>  
> -	if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_crtc_init_scalers(crtc, pipe_config);
> -
> -		pipe_config->scaler_state.scaler_id = -1;
> -		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> -	}
> -
>  	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
>  	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>  		power_domain_mask |= BIT_ULL(power_domain);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW   state readout
@ 2017-07-20  8:58   ` Jani Nikula
  0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2017-07-20  8:58 UTC (permalink / raw)
  To: Imre Deak, intel-gfx; +Cc: stable

On Thu, 20 Jul 2017, Imre Deak <imre.deak@intel.com> wrote:
> The scaler allocation code depends on a non-zero default value for the
> crtc scaler_id, so make sure we initialize the scaler state accordingly
> even if the crtc is off. This fixes at least an initial YUV420 modeset
> (added in a follow-up patchset by Shashank) when booting with the screen
> off: after the initial HW readout and modeset which enables the scaler a
> subsequent modeset will disable the scaler which isn't properly
> allocated. This results in a funky HW state where the pipe scaler HW
> registers can't be modified and the normally black screen is grey and
> shifted to the right or jitters.
>
> The problem was revealed by Shashank's YUV420 patchset and first
> reported by Ville.
>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chandra Konduru <chandra.konduru@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: <stable@vger.kernel.org> # 4.11.x
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
> Signed-off-by: Imre Deak <imre.deak@intel.com>
>
> ---
>
> [ Older stable versions need backporting, so that's for a follow-up ]

I thought we'd annotate cc: stable with all the kernels that need the
fix, not according to where the fix applies as-is. In this case, it
would be v4.2+, right?

BR,
Jani.



> ---
>  drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7774f3465fbc..8a38e64b1931 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	u64 power_domain_mask;
>  	bool active;
>  
> +	if (INTEL_GEN(dev_priv) >= 9) {
> +		intel_crtc_init_scalers(crtc, pipe_config);
> +
> +		pipe_config->scaler_state.scaler_id = -1;
> +		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> +	}
> +
>  	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>  		return false;
> @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	pipe_config->gamma_mode =
>  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>  
> -	if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_crtc_init_scalers(crtc, pipe_config);
> -
> -		pipe_config->scaler_state.scaler_id = -1;
> -		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> -	}
> -
>  	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
>  	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>  		power_domain_mask |= BIT_ULL(power_domain);

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-20  8:58   ` [Intel-gfx] " Jani Nikula
@ 2017-07-20  9:25     ` Imre Deak
  -1 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-20  9:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jani Nikula; +Cc: intel-gfx, stable

On Thu, Jul 20, 2017 at 11:58:35AM +0300, Jani Nikula wrote:
> On Thu, 20 Jul 2017, Imre Deak <imre.deak@intel.com> wrote:
> > The scaler allocation code depends on a non-zero default value for the
> > crtc scaler_id, so make sure we initialize the scaler state accordingly
> > even if the crtc is off. This fixes at least an initial YUV420 modeset
> > (added in a follow-up patchset by Shashank) when booting with the screen
> > off: after the initial HW readout and modeset which enables the scaler a
> > subsequent modeset will disable the scaler which isn't properly
> > allocated. This results in a funky HW state where the pipe scaler HW
> > registers can't be modified and the normally black screen is grey and
> > shifted to the right or jitters.
> >
> > The problem was revealed by Shashank's YUV420 patchset and first
> > reported by Ville.
> >
> > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: <stable@vger.kernel.org> # 4.11.x
> > Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >
> > ---
> >
> > [ Older stable versions need backporting, so that's for a follow-up ]
> 
> I thought we'd annotate cc: stable with all the kernels that need the
> fix, not according to where the fix applies as-is. In this case, it
> would be v4.2+, right?

Hm, not sure. I know that this won't apply before 4.11 and I will have
to send a backported version anyway. So wanted to save a redundant turn
around after the automatic cherry picking to those stable versions
fail.

Greg, what's the proper tag in this case?

Thanks,
Imre

> 
> BR,
> Jani.
> 
> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7774f3465fbc..8a38e64b1931 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  	u64 power_domain_mask;
> >  	bool active;
> >  
> > +	if (INTEL_GEN(dev_priv) >= 9) {
> > +		intel_crtc_init_scalers(crtc, pipe_config);
> > +
> > +		pipe_config->scaler_state.scaler_id = -1;
> > +		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> > +	}
> > +
> >  	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> >  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> >  		return false;
> > @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  	pipe_config->gamma_mode =
> >  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
> >  
> > -	if (INTEL_GEN(dev_priv) >= 9) {
> > -		intel_crtc_init_scalers(crtc, pipe_config);
> > -
> > -		pipe_config->scaler_state.scaler_id = -1;
> > -		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> > -	}
> > -
> >  	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> >  	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
> >  		power_domain_mask |= BIT_ULL(power_domain);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
@ 2017-07-20  9:25     ` Imre Deak
  0 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-20  9:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jani Nikula; +Cc: intel-gfx, stable

On Thu, Jul 20, 2017 at 11:58:35AM +0300, Jani Nikula wrote:
> On Thu, 20 Jul 2017, Imre Deak <imre.deak@intel.com> wrote:
> > The scaler allocation code depends on a non-zero default value for the
> > crtc scaler_id, so make sure we initialize the scaler state accordingly
> > even if the crtc is off. This fixes at least an initial YUV420 modeset
> > (added in a follow-up patchset by Shashank) when booting with the screen
> > off: after the initial HW readout and modeset which enables the scaler a
> > subsequent modeset will disable the scaler which isn't properly
> > allocated. This results in a funky HW state where the pipe scaler HW
> > registers can't be modified and the normally black screen is grey and
> > shifted to the right or jitters.
> >
> > The problem was revealed by Shashank's YUV420 patchset and first
> > reported by Ville.
> >
> > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: <stable@vger.kernel.org> # 4.11.x
> > Reported-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >
> > ---
> >
> > [ Older stable versions need backporting, so that's for a follow-up ]
> 
> I thought we'd annotate cc: stable with all the kernels that need the
> fix, not according to where the fix applies as-is. In this case, it
> would be v4.2+, right?

Hm, not sure. I know that this won't apply before 4.11 and I will have
to send a backported version anyway. So wanted to save a redundant turn
around after the automatic cherry picking to those stable versions
fail.

Greg, what's the proper tag in this case?

Thanks,
Imre

> 
> BR,
> Jani.
> 
> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7774f3465fbc..8a38e64b1931 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  	u64 power_domain_mask;
> >  	bool active;
> >  
> > +	if (INTEL_GEN(dev_priv) >= 9) {
> > +		intel_crtc_init_scalers(crtc, pipe_config);
> > +
> > +		pipe_config->scaler_state.scaler_id = -1;
> > +		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> > +	}
> > +
> >  	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> >  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> >  		return false;
> > @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  	pipe_config->gamma_mode =
> >  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
> >  
> > -	if (INTEL_GEN(dev_priv) >= 9) {
> > -		intel_crtc_init_scalers(crtc, pipe_config);
> > -
> > -		pipe_config->scaler_state.scaler_id = -1;
> > -		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> > -	}
> > -
> >  	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> >  	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
> >  		power_domain_mask |= BIT_ULL(power_domain);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 26+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-19 22:50 [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Imre Deak
                   ` (2 preceding siblings ...)
  2017-07-20  8:58   ` [Intel-gfx] " Jani Nikula
@ 2017-07-20  9:33 ` Patchwork
  2017-07-20 11:28   ` Imre Deak
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2017-07-20  9:33 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Fix scaler init during CRTC HW state readout
URL   : https://patchwork.freedesktop.org/series/27607/
State : success

== Summary ==

Series 27607v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/27607/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900) fdo#101705

fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:455s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:425s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:357s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:539s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:510s
fi-byt-j1900     total:279  pass:255  dwarn:0   dfail:0   fail:0   skip:24  time:492s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:492s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:598s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:439s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:417s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:414s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:505s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:461s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:577s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:585s
fi-pnv-d510      total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  time:559s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:460s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:586s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:468s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:480s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:434s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:474s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:542s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:406s

0fb3fc159dfcd22ce3b81818c3ffb33bcf737936 drm-tip: 2017y-07m-20d-08h-46m-42s UTC integration manifest
5e28a62 drm/i915: Simplify scaler init during CRTC HW readout
dddab95 drm/i915: Fix scaler init during CRTC HW state readout

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5240/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-20  9:25     ` [Intel-gfx] " Imre Deak
@ 2017-07-20  9:41       ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2017-07-20  9:41 UTC (permalink / raw)
  To: Imre Deak; +Cc: Jani Nikula, intel-gfx, stable

On Thu, Jul 20, 2017 at 12:25:30PM +0300, Imre Deak wrote:
> On Thu, Jul 20, 2017 at 11:58:35AM +0300, Jani Nikula wrote:
> > On Thu, 20 Jul 2017, Imre Deak <imre.deak@intel.com> wrote:
> > > The scaler allocation code depends on a non-zero default value for the
> > > crtc scaler_id, so make sure we initialize the scaler state accordingly
> > > even if the crtc is off. This fixes at least an initial YUV420 modeset
> > > (added in a follow-up patchset by Shashank) when booting with the screen
> > > off: after the initial HW readout and modeset which enables the scaler a
> > > subsequent modeset will disable the scaler which isn't properly
> > > allocated. This results in a funky HW state where the pipe scaler HW
> > > registers can't be modified and the normally black screen is grey and
> > > shifted to the right or jitters.
> > >
> > > The problem was revealed by Shashank's YUV420 patchset and first
> > > reported by Ville.
> > >
> > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Cc: <stable@vger.kernel.org> # 4.11.x
> > > Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > >
> > > ---
> > >
> > > [ Older stable versions need backporting, so that's for a follow-up ]
> > 
> > I thought we'd annotate cc: stable with all the kernels that need the
> > fix, not according to where the fix applies as-is. In this case, it
> > would be v4.2+, right?
> 
> Hm, not sure. I know that this won't apply before 4.11 and I will have
> to send a backported version anyway. So wanted to save a redundant turn
> around after the automatic cherry picking to those stable versions
> fail.
> 
> Greg, what's the proper tag in this case?

4.2+ and then when you get the "this didn't apply" email, send the
backported patches.  That way the stable maintainers know it "should" be
applied there, but it just doesn't seem to work with the existing patch.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
@ 2017-07-20  9:41       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2017-07-20  9:41 UTC (permalink / raw)
  To: Imre Deak; +Cc: Jani Nikula, intel-gfx, stable

On Thu, Jul 20, 2017 at 12:25:30PM +0300, Imre Deak wrote:
> On Thu, Jul 20, 2017 at 11:58:35AM +0300, Jani Nikula wrote:
> > On Thu, 20 Jul 2017, Imre Deak <imre.deak@intel.com> wrote:
> > > The scaler allocation code depends on a non-zero default value for the
> > > crtc scaler_id, so make sure we initialize the scaler state accordingly
> > > even if the crtc is off. This fixes at least an initial YUV420 modeset
> > > (added in a follow-up patchset by Shashank) when booting with the screen
> > > off: after the initial HW readout and modeset which enables the scaler a
> > > subsequent modeset will disable the scaler which isn't properly
> > > allocated. This results in a funky HW state where the pipe scaler HW
> > > registers can't be modified and the normally black screen is grey and
> > > shifted to the right or jitters.
> > >
> > > The problem was revealed by Shashank's YUV420 patchset and first
> > > reported by Ville.
> > >
> > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Cc: <stable@vger.kernel.org> # 4.11.x
> > > Reported-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > >
> > > ---
> > >
> > > [ Older stable versions need backporting, so that's for a follow-up ]
> > 
> > I thought we'd annotate cc: stable with all the kernels that need the
> > fix, not according to where the fix applies as-is. In this case, it
> > would be v4.2+, right?
> 
> Hm, not sure. I know that this won't apply before 4.11 and I will have
> to send a backported version anyway. So wanted to save a redundant turn
> around after the automatic cherry picking to those stable versions
> fail.
> 
> Greg, what's the proper tag in this case?

4.2+ and then when you get the "this didn't apply" email, send the
backported patches.  That way the stable maintainers know it "should" be
applied there, but it just doesn't seem to work with the existing patch.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-20  9:41       ` Greg Kroah-Hartman
@ 2017-07-20 11:26         ` Imre Deak
  -1 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-20 11:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: intel-gfx, stable

On Thu, Jul 20, 2017 at 11:41:35AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jul 20, 2017 at 12:25:30PM +0300, Imre Deak wrote:
> > On Thu, Jul 20, 2017 at 11:58:35AM +0300, Jani Nikula wrote:
> > > On Thu, 20 Jul 2017, Imre Deak <imre.deak@intel.com> wrote:
> > > > The scaler allocation code depends on a non-zero default value for the
> > > > crtc scaler_id, so make sure we initialize the scaler state accordingly
> > > > even if the crtc is off. This fixes at least an initial YUV420 modeset
> > > > (added in a follow-up patchset by Shashank) when booting with the screen
> > > > off: after the initial HW readout and modeset which enables the scaler a
> > > > subsequent modeset will disable the scaler which isn't properly
> > > > allocated. This results in a funky HW state where the pipe scaler HW
> > > > registers can't be modified and the normally black screen is grey and
> > > > shifted to the right or jitters.
> > > >
> > > > The problem was revealed by Shashank's YUV420 patchset and first
> > > > reported by Ville.
> > > >
> > > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > > Cc: <stable@vger.kernel.org> # 4.11.x
> > > > Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > >
> > > > ---
> > > >
> > > > [ Older stable versions need backporting, so that's for a follow-up ]
> > > 
> > > I thought we'd annotate cc: stable with all the kernels that need the
> > > fix, not according to where the fix applies as-is. In this case, it
> > > would be v4.2+, right?
> > 
> > Hm, not sure. I know that this won't apply before 4.11 and I will have
> > to send a backported version anyway. So wanted to save a redundant turn
> > around after the automatic cherry picking to those stable versions
> > fail.
> > 
> > Greg, what's the proper tag in this case?
> 
> 4.2+ and then when you get the "this didn't apply" email, send the
> backported patches.  That way the stable maintainers know it "should" be
> applied there, but it just doesn't seem to work with the existing patch.

Ok, will resend with that, thanks for catching this and clarifying.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
@ 2017-07-20 11:26         ` Imre Deak
  0 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-20 11:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jani Nikula, intel-gfx, stable

On Thu, Jul 20, 2017 at 11:41:35AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jul 20, 2017 at 12:25:30PM +0300, Imre Deak wrote:
> > On Thu, Jul 20, 2017 at 11:58:35AM +0300, Jani Nikula wrote:
> > > On Thu, 20 Jul 2017, Imre Deak <imre.deak@intel.com> wrote:
> > > > The scaler allocation code depends on a non-zero default value for the
> > > > crtc scaler_id, so make sure we initialize the scaler state accordingly
> > > > even if the crtc is off. This fixes at least an initial YUV420 modeset
> > > > (added in a follow-up patchset by Shashank) when booting with the screen
> > > > off: after the initial HW readout and modeset which enables the scaler a
> > > > subsequent modeset will disable the scaler which isn't properly
> > > > allocated. This results in a funky HW state where the pipe scaler HW
> > > > registers can't be modified and the normally black screen is grey and
> > > > shifted to the right or jitters.
> > > >
> > > > The problem was revealed by Shashank's YUV420 patchset and first
> > > > reported by Ville.
> > > >
> > > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > > Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > > Cc: <stable@vger.kernel.org> # 4.11.x
> > > > Reported-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > > Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > >
> > > > ---
> > > >
> > > > [ Older stable versions need backporting, so that's for a follow-up ]
> > > 
> > > I thought we'd annotate cc: stable with all the kernels that need the
> > > fix, not according to where the fix applies as-is. In this case, it
> > > would be v4.2+, right?
> > 
> > Hm, not sure. I know that this won't apply before 4.11 and I will have
> > to send a backported version anyway. So wanted to save a redundant turn
> > around after the automatic cherry picking to those stable versions
> > fail.
> > 
> > Greg, what's the proper tag in this case?
> 
> 4.2+ and then when you get the "this didn't apply" email, send the
> backported patches.  That way the stable maintainers know it "should" be
> applied there, but it just doesn't seem to work with the existing patch.

Ok, will resend with that, thanks for catching this and clarifying.

--Imre

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-19 22:50 [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Imre Deak
@ 2017-07-20 11:28   ` Imre Deak
  2017-07-19 23:09 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Fix scaler init during CRTC HW state readout Patchwork
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-20 11:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula, stable

The scaler allocation code depends on a non-zero default value for the
crtc scaler_id, so make sure we initialize the scaler state accordingly
even if the crtc is off. This fixes at least an initial YUV420 modeset
(added in a follow-up patchset by Shashank) when booting with the screen
off: after the initial HW readout and modeset which enables the scaler a
subsequent modeset will disable the scaler which isn't properly
allocated. This results in a funky HW state where the pipe scaler HW
registers can't be modified and the normally black screen is grey and
shifted to the right or jitters.

The problem was revealed by Shashank's YUV420 patchset and first
reported by Ville.

v2:
- In the stable tag also include versions which need backporting (Jani)

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chandra Konduru <chandra.konduru@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: <stable@vger.kernel.org> # 4.2.x
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
Signed-off-by: Imre Deak <imre.deak@intel.com>

---

[ Stable versions before 4.11 need backporting, so that's for a follow-up ]
---
 drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7774f3465fbc..8a38e64b1931 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	u64 power_domain_mask;
 	bool active;
 
+	if (INTEL_GEN(dev_priv) >= 9) {
+		intel_crtc_init_scalers(crtc, pipe_config);
+
+		pipe_config->scaler_state.scaler_id = -1;
+		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
+	}
+
 	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
@@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	pipe_config->gamma_mode =
 		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
 
-	if (INTEL_GEN(dev_priv) >= 9) {
-		intel_crtc_init_scalers(crtc, pipe_config);
-
-		pipe_config->scaler_state.scaler_id = -1;
-		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
-	}
-
 	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
 	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
 		power_domain_mask |= BIT_ULL(power_domain);
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 1/2] drm/i915: Fix scaler init during CRTC HW state readout
@ 2017-07-20 11:28   ` Imre Deak
  0 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-20 11:28 UTC (permalink / raw)
  To: intel-gfx
  Cc: Jani Nikula, Shashank Sharma, Ville Syrjälä,
	Chandra Konduru, Matt Roper, stable

The scaler allocation code depends on a non-zero default value for the
crtc scaler_id, so make sure we initialize the scaler state accordingly
even if the crtc is off. This fixes at least an initial YUV420 modeset
(added in a follow-up patchset by Shashank) when booting with the screen
off: after the initial HW readout and modeset which enables the scaler a
subsequent modeset will disable the scaler which isn't properly
allocated. This results in a funky HW state where the pipe scaler HW
registers can't be modified and the normally black screen is grey and
shifted to the right or jitters.

The problem was revealed by Shashank's YUV420 patchset and first
reported by Ville.

v2:
- In the stable tag also include versions which need backporting (Jani)

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chandra Konduru <chandra.konduru@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: <stable@vger.kernel.org> # 4.2.x
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
Signed-off-by: Imre Deak <imre.deak@intel.com>

---

[ Stable versions before 4.11 need backporting, so that's for a follow-up ]
---
 drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7774f3465fbc..8a38e64b1931 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	u64 power_domain_mask;
 	bool active;
 
+	if (INTEL_GEN(dev_priv) >= 9) {
+		intel_crtc_init_scalers(crtc, pipe_config);
+
+		pipe_config->scaler_state.scaler_id = -1;
+		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
+	}
+
 	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
@@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	pipe_config->gamma_mode =
 		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
 
-	if (INTEL_GEN(dev_priv) >= 9) {
-		intel_crtc_init_scalers(crtc, pipe_config);
-
-		pipe_config->scaler_state.scaler_id = -1;
-		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
-	}
-
 	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
 	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
 		power_domain_mask |= BIT_ULL(power_domain);
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Fix scaler init during CRTC HW state readout (rev2)
  2017-07-19 22:50 [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Imre Deak
                   ` (4 preceding siblings ...)
  2017-07-20 11:28   ` Imre Deak
@ 2017-07-20 12:35 ` Patchwork
  2017-07-21 15:03   ` Imre Deak
  2017-07-20 14:49 ` [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Sharma, Shashank
  6 siblings, 1 reply; 26+ messages in thread
From: Patchwork @ 2017-07-20 12:35 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915: Fix scaler init during CRTC HW state readout (rev2)
URL   : https://patchwork.freedesktop.org/series/27607/
State : success

== Summary ==

Series 27607v2 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/27607/revisions/2/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l) fdo#101781

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:442s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:427s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:350s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:540s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:513s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:489s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:482s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:598s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:435s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:419s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:415s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:507s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:574s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:586s
fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:567s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:459s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:584s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:466s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:478s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:435s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:480s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:544s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:403s

948c3bddb3333e7245970c684749ba5d72370cd2 drm-tip: 2017y-07m-20d-10h-52m-43s UTC integration manifest
ad6c516 drm/i915: Simplify scaler init during CRTC HW readout
8915378 drm/i915: Fix scaler init during CRTC HW state readout

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5246/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-19 22:50 [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Imre Deak
                   ` (5 preceding siblings ...)
  2017-07-20 12:35 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Fix scaler init during CRTC HW state readout (rev2) Patchwork
@ 2017-07-20 14:49 ` Sharma, Shashank
  2017-07-21 12:11   ` [Intel-gfx] " Mahesh Kumar
  6 siblings, 1 reply; 26+ messages in thread
From: Sharma, Shashank @ 2017-07-20 14:49 UTC (permalink / raw)
  To: Imre Deak, intel-gfx
  Cc: Ville Syrjälä, Chandra Konduru, Matt Roper, stable

Acked-by: Shashank Sharma <shashank.sharma@intel.com>

Regards
Shashank
On 7/20/2017 4:20 AM, Imre Deak wrote:
> The scaler allocation code depends on a non-zero default value for the
> crtc scaler_id, so make sure we initialize the scaler state accordingly
> even if the crtc is off. This fixes at least an initial YUV420 modeset
> (added in a follow-up patchset by Shashank) when booting with the screen
> off: after the initial HW readout and modeset which enables the scaler a
> subsequent modeset will disable the scaler which isn't properly
> allocated. This results in a funky HW state where the pipe scaler HW
> registers can't be modified and the normally black screen is grey and
> shifted to the right or jitters.
>
> The problem was revealed by Shashank's YUV420 patchset and first
> reported by Ville.
>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chandra Konduru <chandra.konduru@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: <stable@vger.kernel.org> # 4.11.x
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared scalers")
> Signed-off-by: Imre Deak <imre.deak@intel.com>
>
> ---
>
> [ Older stable versions need backporting, so that's for a follow-up ]
> ---
>   drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7774f3465fbc..8a38e64b1931 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>   	u64 power_domain_mask;
>   	bool active;
>   
> +	if (INTEL_GEN(dev_priv) >= 9) {
> +		intel_crtc_init_scalers(crtc, pipe_config);
> +
> +		pipe_config->scaler_state.scaler_id = -1;
> +		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> +	}
> +
>   	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>   	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>   		return false;
> @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>   	pipe_config->gamma_mode =
>   		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>   
> -	if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_crtc_init_scalers(crtc, pipe_config);
> -
> -		pipe_config->scaler_state.scaler_id = -1;
> -		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> -	}
> -
>   	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
>   	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>   		power_domain_mask |= BIT_ULL(power_domain);

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-20 14:49 ` [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Sharma, Shashank
@ 2017-07-21 12:11   ` Mahesh Kumar
  2017-07-21 12:50       ` [Intel-gfx] " Imre Deak
  0 siblings, 1 reply; 26+ messages in thread
From: Mahesh Kumar @ 2017-07-21 12:11 UTC (permalink / raw)
  To: Sharma, Shashank, Imre Deak, intel-gfx; +Cc: stable

Hi,


On Thursday 20 July 2017 08:19 PM, Sharma, Shashank wrote:
> Acked-by: Shashank Sharma <shashank.sharma@intel.com>
>
> Regards
> Shashank
> On 7/20/2017 4:20 AM, Imre Deak wrote:
>> The scaler allocation code depends on a non-zero default value for the
>> crtc scaler_id, so make sure we initialize the scaler state accordingly
>> even if the crtc is off. This fixes at least an initial YUV420 modeset
>> (added in a follow-up patchset by Shashank) when booting with the screen
>> off: after the initial HW readout and modeset which enables the scaler a
>> subsequent modeset will disable the scaler which isn't properly
>> allocated. This results in a funky HW state where the pipe scaler HW
>> registers can't be modified and the normally black screen is grey and
>> shifted to the right or jitters.
>>
>> The problem was revealed by Shashank's YUV420 patchset and first
>> reported by Ville.
>>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Chandra Konduru <chandra.konduru@intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: <stable@vger.kernel.org> # 4.11.x
>> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared 
>> scalers")
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>
>> ---
>>
>> [ Older stable versions need backporting, so that's for a follow-up ]
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 7774f3465fbc..8a38e64b1931 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct 
>> intel_crtc *crtc,
>>       u64 power_domain_mask;
>>       bool active;
>>   +    if (INTEL_GEN(dev_priv) >= 9) {
>> +        intel_crtc_init_scalers(crtc, pipe_config);
>> +
>> +        pipe_config->scaler_state.scaler_id = -1;
>> +        pipe_config->scaler_state.scaler_users &= ~(1 << 
>> SKL_CRTC_INDEX);
We are already setting scaler_id to -1 during init_scalers & doing 
memset over intel_crtc_state in intel_modeset_readout_hw_state, So IMO 
above 2 lines are unnecessary..
Should not we reading actual HW state of scaler here instead of just 
clearing it.

-Mahesh
>> +    }
>> +
>>       power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>>       if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>>           return false;
>> @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct 
>> intel_crtc *crtc,
>>       pipe_config->gamma_mode =
>>           I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>>   -    if (INTEL_GEN(dev_priv) >= 9) {
>> -        intel_crtc_init_scalers(crtc, pipe_config);
>> -
>> -        pipe_config->scaler_state.scaler_id = -1;
>> -        pipe_config->scaler_state.scaler_users &= ~(1 << 
>> SKL_CRTC_INDEX);
>> -    }
>> -
>>       power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
>>       if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>>           power_domain_mask |= BIT_ULL(power_domain);
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-21 12:11   ` [Intel-gfx] " Mahesh Kumar
@ 2017-07-21 12:50       ` Imre Deak
  0 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-21 12:50 UTC (permalink / raw)
  To: Mahesh Kumar; +Cc: intel-gfx, stable

On Fri, Jul 21, 2017 at 05:41:13PM +0530, Mahesh Kumar wrote:
> Hi,
> 
> 
> On Thursday 20 July 2017 08:19 PM, Sharma, Shashank wrote:
> > Acked-by: Shashank Sharma <shashank.sharma@intel.com>
> > 
> > Regards
> > Shashank
> > On 7/20/2017 4:20 AM, Imre Deak wrote:
> > > The scaler allocation code depends on a non-zero default value for the
> > > crtc scaler_id, so make sure we initialize the scaler state accordingly
> > > even if the crtc is off. This fixes at least an initial YUV420 modeset
> > > (added in a follow-up patchset by Shashank) when booting with the screen
> > > off: after the initial HW readout and modeset which enables the scaler a
> > > subsequent modeset will disable the scaler which isn't properly
> > > allocated. This results in a funky HW state where the pipe scaler HW
> > > registers can't be modified and the normally black screen is grey and
> > > shifted to the right or jitters.
> > > 
> > > The problem was revealed by Shashank's YUV420 patchset and first
> > > reported by Ville.
> > > 
> > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Cc: <stable@vger.kernel.org> # 4.11.x
> > > Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared
> > > scalers")
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > 
> > > ---
> > > 
> > > [ Older stable versions need backporting, so that's for a follow-up ]
> > > ---
> > >   drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
> > >   1 file changed, 7 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 7774f3465fbc..8a38e64b1931 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct
> > > intel_crtc *crtc,
> > >       u64 power_domain_mask;
> > >       bool active;
> > >   +    if (INTEL_GEN(dev_priv) >= 9) {
> > > +        intel_crtc_init_scalers(crtc, pipe_config);
> > > +
> > > +        pipe_config->scaler_state.scaler_id = -1;
> > > +        pipe_config->scaler_state.scaler_users &= ~(1 <<
> > > SKL_CRTC_INDEX);
> We are already setting scaler_id to -1 during init_scalers & doing memset
> over intel_crtc_state in intel_modeset_readout_hw_state, So IMO above 2
> lines are unnecessary..

They are removed in the next patch. I didn't want to remove them in one
go, since for stable I wanted to keep the changes minimal.

> Should not we reading actual HW state of scaler here instead of just
> clearing it.

We will read the scaler state later after determining that the crtc is
on. If it's off the scaler is also off matching the default state we set
here.

> 
> -Mahesh
> > > +    }
> > > +
> > >       power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> > >       if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> > >           return false;
> > > @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct
> > > intel_crtc *crtc,
> > >       pipe_config->gamma_mode =
> > >           I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
> > >   -    if (INTEL_GEN(dev_priv) >= 9) {
> > > -        intel_crtc_init_scalers(crtc, pipe_config);
> > > -
> > > -        pipe_config->scaler_state.scaler_id = -1;
> > > -        pipe_config->scaler_state.scaler_users &= ~(1 <<
> > > SKL_CRTC_INDEX);
> > > -    }
> > > -
> > >       power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> > >       if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
> > >           power_domain_mask |= BIT_ULL(power_domain);
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
@ 2017-07-21 12:50       ` Imre Deak
  0 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-21 12:50 UTC (permalink / raw)
  To: Mahesh Kumar; +Cc: Sharma, Shashank, intel-gfx, stable

On Fri, Jul 21, 2017 at 05:41:13PM +0530, Mahesh Kumar wrote:
> Hi,
> 
> 
> On Thursday 20 July 2017 08:19 PM, Sharma, Shashank wrote:
> > Acked-by: Shashank Sharma <shashank.sharma@intel.com>
> > 
> > Regards
> > Shashank
> > On 7/20/2017 4:20 AM, Imre Deak wrote:
> > > The scaler allocation code depends on a non-zero default value for the
> > > crtc scaler_id, so make sure we initialize the scaler state accordingly
> > > even if the crtc is off. This fixes at least an initial YUV420 modeset
> > > (added in a follow-up patchset by Shashank) when booting with the screen
> > > off: after the initial HW readout and modeset which enables the scaler a
> > > subsequent modeset will disable the scaler which isn't properly
> > > allocated. This results in a funky HW state where the pipe scaler HW
> > > registers can't be modified and the normally black screen is grey and
> > > shifted to the right or jitters.
> > > 
> > > The problem was revealed by Shashank's YUV420 patchset and first
> > > reported by Ville.
> > > 
> > > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > > Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Cc: <stable@vger.kernel.org> # 4.11.x
> > > Reported-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared
> > > scalers")
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > 
> > > ---
> > > 
> > > [ Older stable versions need backporting, so that's for a follow-up ]
> > > ---
> > >   drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
> > >   1 file changed, 7 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 7774f3465fbc..8a38e64b1931 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct
> > > intel_crtc *crtc,
> > >       u64 power_domain_mask;
> > >       bool active;
> > >   +    if (INTEL_GEN(dev_priv) >= 9) {
> > > +        intel_crtc_init_scalers(crtc, pipe_config);
> > > +
> > > +        pipe_config->scaler_state.scaler_id = -1;
> > > +        pipe_config->scaler_state.scaler_users &= ~(1 <<
> > > SKL_CRTC_INDEX);
> We are already setting scaler_id to -1 during init_scalers & doing memset
> over intel_crtc_state in intel_modeset_readout_hw_state, So IMO above 2
> lines are unnecessary..

They are removed in the next patch. I didn't want to remove them in one
go, since for stable I wanted to keep the changes minimal.

> Should not we reading actual HW state of scaler here instead of just
> clearing it.

We will read the scaler state later after determining that the crtc is
on. If it's off the scaler is also off matching the default state we set
here.

> 
> -Mahesh
> > > +    }
> > > +
> > >       power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> > >       if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> > >           return false;
> > > @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct
> > > intel_crtc *crtc,
> > >       pipe_config->gamma_mode =
> > >           I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
> > >   -    if (INTEL_GEN(dev_priv) >= 9) {
> > > -        intel_crtc_init_scalers(crtc, pipe_config);
> > > -
> > > -        pipe_config->scaler_state.scaler_id = -1;
> > > -        pipe_config->scaler_state.scaler_users &= ~(1 <<
> > > SKL_CRTC_INDEX);
> > > -    }
> > > -
> > >       power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> > >       if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
> > >           power_domain_mask |= BIT_ULL(power_domain);
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
  2017-07-21 12:50       ` [Intel-gfx] " Imre Deak
@ 2017-07-21 13:06         ` Mahesh Kumar
  -1 siblings, 0 replies; 26+ messages in thread
From: Mahesh Kumar @ 2017-07-21 13:06 UTC (permalink / raw)
  To: imre.deak; +Cc: intel-gfx, stable

Hi


On Friday 21 July 2017 06:20 PM, Imre Deak wrote:
> On Fri, Jul 21, 2017 at 05:41:13PM +0530, Mahesh Kumar wrote:
>> Hi,
>>
>>
>> On Thursday 20 July 2017 08:19 PM, Sharma, Shashank wrote:
>>> Acked-by: Shashank Sharma <shashank.sharma@intel.com>
>>>
>>> Regards
>>> Shashank
>>> On 7/20/2017 4:20 AM, Imre Deak wrote:
>>>> The scaler allocation code depends on a non-zero default value for the
>>>> crtc scaler_id, so make sure we initialize the scaler state accordingly
>>>> even if the crtc is off. This fixes at least an initial YUV420 modeset
>>>> (added in a follow-up patchset by Shashank) when booting with the screen
>>>> off: after the initial HW readout and modeset which enables the scaler a
>>>> subsequent modeset will disable the scaler which isn't properly
>>>> allocated. This results in a funky HW state where the pipe scaler HW
>>>> registers can't be modified and the normally black screen is grey and
>>>> shifted to the right or jitters.
>>>>
>>>> The problem was revealed by Shashank's YUV420 patchset and first
>>>> reported by Ville.
>>>>
>>>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Cc: Chandra Konduru <chandra.konduru@intel.com>
>>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>>> Cc: <stable@vger.kernel.org> # 4.11.x
>>>> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared
>>>> scalers")
>>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>>>
>>>> ---
>>>>
>>>> [ Older stable versions need backporting, so that's for a follow-up ]
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>>>>    1 file changed, 7 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>>> b/drivers/gpu/drm/i915/intel_display.c
>>>> index 7774f3465fbc..8a38e64b1931 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct
>>>> intel_crtc *crtc,
>>>>        u64 power_domain_mask;
>>>>        bool active;
>>>>    +    if (INTEL_GEN(dev_priv) >= 9) {
>>>> +        intel_crtc_init_scalers(crtc, pipe_config);
>>>> +
>>>> +        pipe_config->scaler_state.scaler_id = -1;
>>>> +        pipe_config->scaler_state.scaler_users &= ~(1 <<
>>>> SKL_CRTC_INDEX);
>> We are already setting scaler_id to -1 during init_scalers & doing memset
>> over intel_crtc_state in intel_modeset_readout_hw_state, So IMO above 2
>> lines are unnecessary..
> They are removed in the next patch. I didn't want to remove them in one
> go, since for stable I wanted to keep the changes minimal.
OK, l'm fine with it.
Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>
>> Should not we reading actual HW state of scaler here instead of just
>> clearing it.
> We will read the scaler state later after determining that the crtc is
> on. If it's off the scaler is also off matching the default state we set
> here.
>
>> -Mahesh
>>>> +    }
>>>> +
>>>>        power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>>>>        if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>>>>            return false;
>>>> @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct
>>>> intel_crtc *crtc,
>>>>        pipe_config->gamma_mode =
>>>>            I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>>>>    -    if (INTEL_GEN(dev_priv) >= 9) {
>>>> -        intel_crtc_init_scalers(crtc, pipe_config);
>>>> -
>>>> -        pipe_config->scaler_state.scaler_id = -1;
>>>> -        pipe_config->scaler_state.scaler_users &= ~(1 <<
>>>> SKL_CRTC_INDEX);
>>>> -    }
>>>> -
>>>>        power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
>>>>        if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>>>>            power_domain_mask |= BIT_ULL(power_domain);
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout
@ 2017-07-21 13:06         ` Mahesh Kumar
  0 siblings, 0 replies; 26+ messages in thread
From: Mahesh Kumar @ 2017-07-21 13:06 UTC (permalink / raw)
  To: imre.deak; +Cc: Sharma, Shashank, intel-gfx, stable

Hi


On Friday 21 July 2017 06:20 PM, Imre Deak wrote:
> On Fri, Jul 21, 2017 at 05:41:13PM +0530, Mahesh Kumar wrote:
>> Hi,
>>
>>
>> On Thursday 20 July 2017 08:19 PM, Sharma, Shashank wrote:
>>> Acked-by: Shashank Sharma <shashank.sharma@intel.com>
>>>
>>> Regards
>>> Shashank
>>> On 7/20/2017 4:20 AM, Imre Deak wrote:
>>>> The scaler allocation code depends on a non-zero default value for the
>>>> crtc scaler_id, so make sure we initialize the scaler state accordingly
>>>> even if the crtc is off. This fixes at least an initial YUV420 modeset
>>>> (added in a follow-up patchset by Shashank) when booting with the screen
>>>> off: after the initial HW readout and modeset which enables the scaler a
>>>> subsequent modeset will disable the scaler which isn't properly
>>>> allocated. This results in a funky HW state where the pipe scaler HW
>>>> registers can't be modified and the normally black screen is grey and
>>>> shifted to the right or jitters.
>>>>
>>>> The problem was revealed by Shashank's YUV420 patchset and first
>>>> reported by Ville.
>>>>
>>>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Cc: Chandra Konduru <chandra.konduru@intel.com>
>>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>>> Cc: <stable@vger.kernel.org> # 4.11.x
>>>> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Fixes: a1b2278e4dfc ("drm/i915: skylake panel fitting using shared
>>>> scalers")
>>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>>>
>>>> ---
>>>>
>>>> [ Older stable versions need backporting, so that's for a follow-up ]
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_display.c | 14 +++++++-------
>>>>    1 file changed, 7 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>>> b/drivers/gpu/drm/i915/intel_display.c
>>>> index 7774f3465fbc..8a38e64b1931 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -9132,6 +9132,13 @@ static bool haswell_get_pipe_config(struct
>>>> intel_crtc *crtc,
>>>>        u64 power_domain_mask;
>>>>        bool active;
>>>>    +    if (INTEL_GEN(dev_priv) >= 9) {
>>>> +        intel_crtc_init_scalers(crtc, pipe_config);
>>>> +
>>>> +        pipe_config->scaler_state.scaler_id = -1;
>>>> +        pipe_config->scaler_state.scaler_users &= ~(1 <<
>>>> SKL_CRTC_INDEX);
>> We are already setting scaler_id to -1 during init_scalers & doing memset
>> over intel_crtc_state in intel_modeset_readout_hw_state, So IMO above 2
>> lines are unnecessary..
> They are removed in the next patch. I didn't want to remove them in one
> go, since for stable I wanted to keep the changes minimal.
OK, l'm fine with it.
Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>
>> Should not we reading actual HW state of scaler here instead of just
>> clearing it.
> We will read the scaler state later after determining that the crtc is
> on. If it's off the scaler is also off matching the default state we set
> here.
>
>> -Mahesh
>>>> +    }
>>>> +
>>>>        power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>>>>        if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>>>>            return false;
>>>> @@ -9160,13 +9167,6 @@ static bool haswell_get_pipe_config(struct
>>>> intel_crtc *crtc,
>>>>        pipe_config->gamma_mode =
>>>>            I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>>>>    -    if (INTEL_GEN(dev_priv) >= 9) {
>>>> -        intel_crtc_init_scalers(crtc, pipe_config);
>>>> -
>>>> -        pipe_config->scaler_state.scaler_id = -1;
>>>> -        pipe_config->scaler_state.scaler_users &= ~(1 <<
>>>> SKL_CRTC_INDEX);
>>>> -    }
>>>> -
>>>>        power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
>>>>        if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>>>>            power_domain_mask |= BIT_ULL(power_domain);
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout
  2017-07-19 22:50 ` [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout Imre Deak
@ 2017-07-21 13:14   ` Mahesh Kumar
  2017-07-21 13:26     ` Imre Deak
  0 siblings, 1 reply; 26+ messages in thread
From: Mahesh Kumar @ 2017-07-21 13:14 UTC (permalink / raw)
  To: Imre Deak, intel-gfx

Hi,


On Thursday 20 July 2017 04:20 AM, Imre Deak wrote:
> The crtc state starts out being bzero'd, so no need to clear
> scaler_users. Also intel_crtc_init_scalers() knows already which
> platforms have scalers, so no need for the platform check here.
> Similarly intel_crtc_init_scalers() will init scaler_id as required,
> so no need to do it here separately.
>
> Cc: Chandra Konduru <chandra.konduru@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8a38e64b1931..7210f418e9c0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9132,12 +9132,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>   	u64 power_domain_mask;
>   	bool active;
>   
> -	if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_crtc_init_scalers(crtc, pipe_config);
> -
> -		pipe_config->scaler_state.scaler_id = -1;
> -		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> -	}
> +	intel_crtc_init_scalers(crtc, pipe_config);
If we are removing gen check, then IMHO we should initialize "scaler_id 
= -1" even before !crtc->num_scalers check in intel_crtc_init_scalers 
function, just to make sure scaler state doesn't have wrong value in 
platforms where we don't have any scalers.

-Mahesh
>   
>   	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>   	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout
  2017-07-21 13:14   ` Mahesh Kumar
@ 2017-07-21 13:26     ` Imre Deak
  2017-07-21 14:06       ` Mahesh Kumar
  0 siblings, 1 reply; 26+ messages in thread
From: Imre Deak @ 2017-07-21 13:26 UTC (permalink / raw)
  To: Mahesh Kumar; +Cc: intel-gfx

On Fri, Jul 21, 2017 at 06:44:24PM +0530, Mahesh Kumar wrote:
> Hi,
> 
> 
> On Thursday 20 July 2017 04:20 AM, Imre Deak wrote:
> > The crtc state starts out being bzero'd, so no need to clear
> > scaler_users. Also intel_crtc_init_scalers() knows already which
> > platforms have scalers, so no need for the platform check here.
> > Similarly intel_crtc_init_scalers() will init scaler_id as required,
> > so no need to do it here separately.
> > 
> > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 7 +------
> >   1 file changed, 1 insertion(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 8a38e64b1931..7210f418e9c0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9132,12 +9132,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >   	u64 power_domain_mask;
> >   	bool active;
> > -	if (INTEL_GEN(dev_priv) >= 9) {
> > -		intel_crtc_init_scalers(crtc, pipe_config);
> > -
> > -		pipe_config->scaler_state.scaler_id = -1;
> > -		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> > -	}
> > +	intel_crtc_init_scalers(crtc, pipe_config);
> If we are removing gen check, then IMHO we should initialize "scaler_id =
> -1" even before !crtc->num_scalers check in intel_crtc_init_scalers
> function, just to make sure scaler state doesn't have wrong value in
> platforms where we don't have any scalers.

I agree that setting scaler_id to -1 on all platforms is a good idea,
but it's unrelated to removing here the GEN check. intel_crtc_init_scalers()
is already called for all platforms in intel_crtc_init(). So this change won't
make a difference for platforms without scalers: we leave scaler_id zeroed
on those already now (they have num_scalers set to 0). This isn't either
a problem since we allocate scalers only on GEN9+.

So we could do what you suggest but imo as a follow-up.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout
  2017-07-21 13:26     ` Imre Deak
@ 2017-07-21 14:06       ` Mahesh Kumar
  2017-07-21 14:33         ` Imre Deak
  0 siblings, 1 reply; 26+ messages in thread
From: Mahesh Kumar @ 2017-07-21 14:06 UTC (permalink / raw)
  To: imre.deak; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 2478 bytes --]

Hi,


On Friday 21 July 2017 06:56 PM, Imre Deak wrote:
> On Fri, Jul 21, 2017 at 06:44:24PM +0530, Mahesh Kumar wrote:
>> Hi,
>>
>>
>> On Thursday 20 July 2017 04:20 AM, Imre Deak wrote:
>>> The crtc state starts out being bzero'd, so no need to clear
>>> scaler_users. Also intel_crtc_init_scalers() knows already which
>>> platforms have scalers, so no need for the platform check here.
>>> Similarly intel_crtc_init_scalers() will init scaler_id as required,
>>> so no need to do it here separately.
>>>
>>> Cc: Chandra Konduru <chandra.konduru@intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_display.c | 7 +------
>>>    1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 8a38e64b1931..7210f418e9c0 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -9132,12 +9132,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>>    	u64 power_domain_mask;
>>>    	bool active;
>>> -	if (INTEL_GEN(dev_priv) >= 9) {
>>> -		intel_crtc_init_scalers(crtc, pipe_config);
>>> -
>>> -		pipe_config->scaler_state.scaler_id = -1;
>>> -		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
>>> -	}
>>> +	intel_crtc_init_scalers(crtc, pipe_config);
>> If we are removing gen check, then IMHO we should initialize "scaler_id =
>> -1" even before !crtc->num_scalers check in intel_crtc_init_scalers
>> function, just to make sure scaler state doesn't have wrong value in
>> platforms where we don't have any scalers.
> I agree that setting scaler_id to -1 on all platforms is a good idea,
> but it's unrelated to removing here the GEN check. intel_crtc_init_scalers()
> is already called for all platforms in intel_crtc_init(). So this change won't
> make a difference for platforms without scalers: we leave scaler_id zeroed
> on those already now (they have num_scalers set to 0). This isn't either
> a problem since we allocate scalers only on GEN9+.
>
> So we could do what you suggest but imo as a follow-up.
sounds good, But if you are going to write a cleanup patch, will that be 
possible for you to do following change also.
     0 indicates no scaler is used & non-zero +ve value indicates used 
scaler id.
in any case

Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com>

-Mahesh
>
> --Imre


[-- Attachment #1.2: Type: text/html, Size: 3650 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout
  2017-07-21 14:06       ` Mahesh Kumar
@ 2017-07-21 14:33         ` Imre Deak
  0 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-21 14:33 UTC (permalink / raw)
  To: Mahesh Kumar; +Cc: intel-gfx

On Fri, Jul 21, 2017 at 07:36:22PM +0530, Mahesh Kumar wrote:
> Hi,
> 
> 
> On Friday 21 July 2017 06:56 PM, Imre Deak wrote:
> > On Fri, Jul 21, 2017 at 06:44:24PM +0530, Mahesh Kumar wrote:
> > > Hi,
> > > 
> > > 
> > > On Thursday 20 July 2017 04:20 AM, Imre Deak wrote:
> > > > The crtc state starts out being bzero'd, so no need to clear
> > > > scaler_users. Also intel_crtc_init_scalers() knows already which
> > > > platforms have scalers, so no need for the platform check here.
> > > > Similarly intel_crtc_init_scalers() will init scaler_id as required,
> > > > so no need to do it here separately.
> > > > 
> > > > Cc: Chandra Konduru <chandra.konduru@intel.com>
> > > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >    drivers/gpu/drm/i915/intel_display.c | 7 +------
> > > >    1 file changed, 1 insertion(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index 8a38e64b1931..7210f418e9c0 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -9132,12 +9132,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> > > >    	u64 power_domain_mask;
> > > >    	bool active;
> > > > -	if (INTEL_GEN(dev_priv) >= 9) {
> > > > -		intel_crtc_init_scalers(crtc, pipe_config);
> > > > -
> > > > -		pipe_config->scaler_state.scaler_id = -1;
> > > > -		pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX);
> > > > -	}
> > > > +	intel_crtc_init_scalers(crtc, pipe_config);
> > >
> > > If we are removing gen check, then IMHO we should initialize "scaler_id =
> > > -1" even before !crtc->num_scalers check in intel_crtc_init_scalers
> > > function, just to make sure scaler state doesn't have wrong value in
> > > platforms where we don't have any scalers.
> >
> > I agree that setting scaler_id to -1 on all platforms is a good idea,
> > but it's unrelated to removing here the GEN check. intel_crtc_init_scalers()
> > is already called for all platforms in intel_crtc_init(). So this change won't
> > make a difference for platforms without scalers: we leave scaler_id zeroed
> > on those already now (they have num_scalers set to 0). This isn't either
> > a problem since we allocate scalers only on GEN9+.
> > 
> > So we could do what you suggest but imo as a follow-up.
>
> sounds good, But if you are going to write a cleanup patch, will that
> be possible for you to do following change also.  0 indicates no
> scaler is used & non-zero +ve value indicates used scaler id.

That's one possibility, or we could just use a helper to initialize the
intel specific part of the state; there could be other fields there with
non-zero default values. This helper could be then used in the few
places where this is done now (or done partially) open-coded.

Also patches are welcome, feel free to follow-up:)

> in any case
> 
> Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> 
> -Mahesh
> > 
> > --Imre
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Fix scaler init during CRTC HW state readout (rev2)
  2017-07-20 12:35 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Fix scaler init during CRTC HW state readout (rev2) Patchwork
@ 2017-07-21 15:03   ` Imre Deak
  0 siblings, 0 replies; 26+ messages in thread
From: Imre Deak @ 2017-07-21 15:03 UTC (permalink / raw)
  To: intel-gfx, Mahesh Kumar, Jani Nikula

On Thu, Jul 20, 2017 at 12:35:27PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [v2,1/2] drm/i915: Fix scaler init during CRTC HW state readout (rev2)
> URL   : https://patchwork.freedesktop.org/series/27607/
> State : success
> 
> == Summary ==
> 
> Series 27607v2 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/27607/revisions/2/mbox/

Thanks for the review, I pushed these to -dinq.

> 
> Test kms_cursor_legacy:
>         Subgroup basic-busy-flip-before-cursor-atomic:
>                 fail       -> PASS       (fi-snb-2600) fdo#100215
> Test kms_flip:
>         Subgroup basic-flip-vs-modeset:
>                 pass       -> SKIP       (fi-skl-x1585l) fdo#101781
> 
> fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
> fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
> 
> fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:442s
> fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:427s
> fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:350s
> fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:540s
> fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:513s
> fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:489s
> fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:482s
> fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:598s
> fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:435s
> fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:419s
> fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:415s
> fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:507s
> fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:476s
> fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:466s
> fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:574s
> fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:586s
> fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:567s
> fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:459s
> fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:584s
> fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:466s
> fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:478s
> fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:435s
> fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:480s
> fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:544s
> fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:403s
> 
> 948c3bddb3333e7245970c684749ba5d72370cd2 drm-tip: 2017y-07m-20d-10h-52m-43s UTC integration manifest
> ad6c516 drm/i915: Simplify scaler init during CRTC HW readout
> 8915378 drm/i915: Fix scaler init during CRTC HW state readout
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5246/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2017-07-21 15:03 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-19 22:50 [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Imre Deak
2017-07-19 22:50 ` [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout Imre Deak
2017-07-21 13:14   ` Mahesh Kumar
2017-07-21 13:26     ` Imre Deak
2017-07-21 14:06       ` Mahesh Kumar
2017-07-21 14:33         ` Imre Deak
2017-07-19 23:09 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Fix scaler init during CRTC HW state readout Patchwork
2017-07-20  8:58 ` [PATCH 1/2] " Jani Nikula
2017-07-20  8:58   ` [Intel-gfx] " Jani Nikula
2017-07-20  9:25   ` Imre Deak
2017-07-20  9:25     ` [Intel-gfx] " Imre Deak
2017-07-20  9:41     ` Greg Kroah-Hartman
2017-07-20  9:41       ` Greg Kroah-Hartman
2017-07-20 11:26       ` Imre Deak
2017-07-20 11:26         ` [Intel-gfx] " Imre Deak
2017-07-20  9:33 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2017-07-20 11:28 ` [PATCH v2 1/2] " Imre Deak
2017-07-20 11:28   ` Imre Deak
2017-07-20 12:35 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915: Fix scaler init during CRTC HW state readout (rev2) Patchwork
2017-07-21 15:03   ` Imre Deak
2017-07-20 14:49 ` [PATCH 1/2] drm/i915: Fix scaler init during CRTC HW state readout Sharma, Shashank
2017-07-21 12:11   ` [Intel-gfx] " Mahesh Kumar
2017-07-21 12:50     ` Imre Deak
2017-07-21 12:50       ` [Intel-gfx] " Imre Deak
2017-07-21 13:06       ` Mahesh Kumar
2017-07-21 13:06         ` [Intel-gfx] " Mahesh Kumar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.