Re: [PATCH] drm/i915: Don't do pre plane update on disabled crtcs

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 14-01-16 om 17:52 schreef Ville Syrjälä:
> On Thu, Jan 14, 2016 at 06:32:10PM +0200, Mika Kuoppala wrote:
>> CI/Bat got following (shortened) trace on byt and also
>> on bsw:
>>
>> ------------[ cut here ]-----------
>> Unclaimed register detected before reading register 0x186500
>> Call Trace:
>>  __unclaimed_reg_debug+0x68/0x80 [i915]
>> vlv_read32+0x2de/0x370 [i915]
>> intel_set_memory_cxsr+0x87/0x1a0 [i915]
>> intel_pre_plane_update+0xb3/0xf0 [i915]
>> intel_atomic_commit+0x3b5/0x17c0 [i915]
>> ...
>> ---[ end trace 6387a0ad001bb39f ]---
>>
>> Fix this by limiting pre plane update only to active crtcs.
>>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=93698
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index aa24f79d85bf..a134a698d97d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13580,9 +13580,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>>  		if (!needs_modeset(crtc->state))
>>  			continue;
>>  
>> -		intel_pre_plane_update(intel_crtc);
>> -
>>  		if (crtc_state->active) {
>> +			intel_pre_plane_update(intel_crtc);
> I think you'll want to deal with the other one too (the one in the crtc
> enable/plane update path). Actually I think the plane update stuff
> should be split into a separate loop from the crtc_enable stuff, but
> that's a separate topic.
>
> Hmm. And there's a post_plane_update there that looks a bit too
> lonely as well. Something really should be done to make this code
> less convoluted. A modeset/plane update shouldn't be this hard to
> get right.
>
>
I understand the idea from this comment, but nothing in pre_plane_update should run when crtc was not active. What function is called that does?

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