Re: [PATCH 1/2] drm/i915: fix the SDE irq dmesg warnings properly

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 25 Nov 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Nov 25, 2015 at 04:47:22PM +0200, Jani Nikula wrote:
>> We had the "The master control interrupt lied (SDE)!" check and error
>> message in place for a long time without any problems, until
>> 
>> commit aaf5ec2e51ab1d9c5e962b4728a1107ed3ff7a3e
>> Author: Sonika Jindal <sonika.jindal@intel.com>
>> Date:   Wed Jul 8 17:07:47 2015 +0530
>> 
>>     drm/i915: Handle HPD when it has actually occurred
>> 
>> caused the errors to start happening. This was bisected and reported,
>> but the error message was silenced in
>> 
>> commit 97e5ed1111dcc5300a0f59a55248cd243937a8ab
>> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Date:   Fri Oct 23 10:56:12 2015 +0200
>> 
>>     drm/i915: shut up gen8+ SDE irq dmesg noise
>> 
>> shooting the messenger while the debugging for why Sonika's commit
>> triggered the errors was still in progress.
>> 
>> It looks like we need to read and acknowledge the PCH_PORT_HOTPLUG
>> register even though the hotplug trigger indicates there isn't a hotplug
>> irq to handle.
>> 
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Sonika Jindal <sonika.jindal@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92084
>> Fixes: aaf5ec2e51ab ("drm/i915: Handle HPD when it has actually occurred")
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index c8ba94968aaf..982951d3153a 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1825,7 +1825,17 @@ static void ibx_hpd_irq_handler(struct drm_device *dev, u32 hotplug_trigger,
>>  	u32 dig_hotplug_reg, pin_mask = 0, long_mask = 0;
>>  
>>  	dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
>> +	if (!hotplug_trigger) {
>> +		u32 mask = PORTA_HOTPLUG_STATUS_MASK |
>> +			PORTD_HOTPLUG_STATUS_MASK |
>> +			PORTC_HOTPLUG_STATUS_MASK |
>> +			PORTB_HOTPLUG_STATUS_MASK;
>> +		dig_hotplug_reg &= ~mask;
>> +	}
>> +
>>  	I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>> +	if (!hotplug_trigger)
>> +		return;
>
> I would add some kind of comment around these parts to explain that
> somehow the PCH doesn't seem to really ack the interrupt to the CPU
> unless we touch the hotplug register.
>
> Otherwise (for both patches)
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pushed to drm-intel-next-queued, with a comment added and the commit
message amended slightly, thanks for the review.

BR,
Jani.



>
> Also not sure if SKL might need something similar as well...
>
>>  
>>  	intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
>>  			   dig_hotplug_reg, hpd,
>> @@ -1840,8 +1850,7 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
>>  	int pipe;
>>  	u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK;
>>  
>> -	if (hotplug_trigger)
>> -		ibx_hpd_irq_handler(dev, hotplug_trigger, hpd_ibx);
>> +	ibx_hpd_irq_handler(dev, hotplug_trigger, hpd_ibx);
>>  
>>  	if (pch_iir & SDE_AUDIO_POWER_MASK) {
>>  		int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK) >>
>> @@ -1934,8 +1943,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>>  	int pipe;
>>  	u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
>>  
>> -	if (hotplug_trigger)
>> -		ibx_hpd_irq_handler(dev, hotplug_trigger, hpd_cpt);
>> +	ibx_hpd_irq_handler(dev, hotplug_trigger, hpd_cpt);
>>  
>>  	if (pch_iir & SDE_AUDIO_POWER_MASK_CPT) {
>>  		int port = ffs((pch_iir & SDE_AUDIO_POWER_MASK_CPT) >>
>> -- 
>> 2.1.4

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