[PATCH] drm/i915: Set best_encoder field of connector_state also when disabling

[PATCH] drm/i915: Set best_encoder field of connector_state also when disabling

[Ander Conselvan de Oliveira]

The best_encoder field of connector_state wasn't properly set when a
connector was being disabled, leading to an incosistent atomic state.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a693129..307c43b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12009,6 +12009,7 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 				connector->encoder = connector->new_encoder;
 		} else {
 			connector_state->crtc = NULL;
+			connector_state->best_encoder = NULL;
 		}
 	}
 	for_each_intel_crtc(dev, crtc) {
-- 
2.1.0

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

Re: [PATCH] drm/i915: Set best_encoder field of connector_state also when disabling

[Daniel Vetter]

On Tue, Mar 31, 2015 at 11:35:00AM +0300, Ander Conselvan de Oliveira wrote:
> The best_encoder field of connector_state wasn't properly set when a
> connector was being disabled, leading to an incosistent atomic state.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

How does this typically blow up, i.e. commit message is a bit too terse
imo. Still, applied to dinf for now.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a693129..307c43b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12009,6 +12009,7 @@ intel_modeset_stage_output_state(struct drm_device *dev,
>  				connector->encoder = connector->new_encoder;
>  		} else {
>  			connector_state->crtc = NULL;
> +			connector_state->best_encoder = NULL;
>  		}
>  	}
>  	for_each_intel_crtc(dev, crtc) {
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Set best_encoder field of connector_state also when disabling

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6101
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  303/303              303/303
SNB                 -1              304/304              303/304
IVB                                  338/338              338/338
BYT                                  287/287              287/287
HSW                                  361/361              361/361
BDW                                  309/309              309/309
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*SNB  igt@gem_pwrite_pread@display-copy-performance      PASS(2)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Set best_encoder field of connector_state also when disabling

[Ander Conselvan De Oliveira]

On Tue, 2015-03-31 at 15:26 +0200, Daniel Vetter wrote:
> On Tue, Mar 31, 2015 at 11:35:00AM +0300, Ander Conselvan de Oliveira wrote:
> > The best_encoder field of connector_state wasn't properly set when a
> > connector was being disabled, leading to an incosistent atomic state.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> 
> How does this typically blow up, i.e. commit message is a bit too terse
> imo. Still, applied to dinf for now.

Yeah, I should have been more verbose on this one.

For now, this doesn't cause anything to blow up, because everywhere
we're using connector_state->best_encoder there is a check for
connector_state->crtc which is properly initialized. I reached the issue
while testing some patches I haven't sent out yet, that remove the usage
of intel_connector->new_encoder from check_digital_port_conflicts(). In
that case, it would be possible to trigger the converted version of the
WARN in that function.

Ander

> Thanks, Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a693129..307c43b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12009,6 +12009,7 @@ intel_modeset_stage_output_state(struct drm_device *dev,
> >  				connector->encoder = connector->new_encoder;
> >  		} else {
> >  			connector_state->crtc = NULL;
> > +			connector_state->best_encoder = NULL;
> >  		}
> >  	}
> >  	for_each_intel_crtc(dev, crtc) {
> > -- 
> > 2.1.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


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

Re: [PATCH] drm/i915: Set best_encoder field of connector_state also when disabling

[Daniel Vetter]

On Wed, Apr 01, 2015 at 11:17:32AM +0300, Ander Conselvan De Oliveira wrote:
> On Tue, 2015-03-31 at 15:26 +0200, Daniel Vetter wrote:
> > On Tue, Mar 31, 2015 at 11:35:00AM +0300, Ander Conselvan de Oliveira wrote:
> > > The best_encoder field of connector_state wasn't properly set when a
> > > connector was being disabled, leading to an incosistent atomic state.
> > > 
> > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > 
> > How does this typically blow up, i.e. commit message is a bit too terse
> > imo. Still, applied to dinf for now.
> 
> Yeah, I should have been more verbose on this one.
> 
> For now, this doesn't cause anything to blow up, because everywhere
> we're using connector_state->best_encoder there is a check for
> connector_state->crtc which is properly initialized. I reached the issue
> while testing some patches I haven't sent out yet, that remove the usage
> of intel_connector->new_encoder from check_digital_port_conflicts(). In
> that case, it would be possible to trigger the converted version of the
> WARN in that function.

Thanks, added and moved the patch from dinf to dinq.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx