* Re: [Intel-gfx] [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-20 9:25 ` Imre Deak
2017-07-20 11:28 ` [PATCH v2 " Imre Deak
2017-07-20 14:49 ` [PATCH " Sharma, Shashank
2 siblings, 1 reply; 10+ 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] 10+ messages in thread* Re: [Intel-gfx] [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
2017-07-20 9:41 ` Greg Kroah-Hartman
0 siblings, 1 reply; 10+ 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] 10+ 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
@ 2017-07-20 9:41 ` Greg Kroah-Hartman
2017-07-20 11:26 ` Imre Deak
0 siblings, 1 reply; 10+ 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] 10+ 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
@ 2017-07-20 11:26 ` Imre Deak
0 siblings, 0 replies; 10+ 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] 10+ 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 8:58 ` [Intel-gfx] " Jani Nikula
@ 2017-07-20 11:28 ` Imre Deak
2017-07-20 14:49 ` [PATCH " Sharma, Shashank
2 siblings, 0 replies; 10+ 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] 10+ 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 ` [Intel-gfx] " Jani Nikula
2017-07-20 11:28 ` [PATCH v2 " Imre Deak
@ 2017-07-20 14:49 ` Sharma, Shashank
2017-07-21 12:11 ` [Intel-gfx] " Mahesh Kumar
2 siblings, 1 reply; 10+ 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] 10+ 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 " Sharma, Shashank
@ 2017-07-21 12:11 ` Mahesh Kumar
2017-07-21 12:50 ` Imre Deak
0 siblings, 1 reply; 10+ 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] 10+ messages in thread* Re: [Intel-gfx] [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
2017-07-21 13:06 ` Mahesh Kumar
0 siblings, 1 reply; 10+ 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] 10+ 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
@ 2017-07-21 13:06 ` Mahesh Kumar
0 siblings, 0 replies; 10+ 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] 10+ messages in thread