* [PATCH 01/11] drm/i915: Clean up various HPD defines
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-17 19:51 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 02/11] drm/i915; Extract intel_hpd_enabled_irqs() ville.syrjala
` (11 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Indent the PORTx_HOTPLUG_... defines appropriately, and fix some space
vs. tab issues.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 72 +++++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6786e94..ed2d150 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5364,15 +5364,17 @@ enum skl_disp_power_wells {
#define CPU_VGACNTRL 0x41000
-#define DIGITAL_PORT_HOTPLUG_CNTRL 0x44030
-#define DIGITAL_PORTA_HOTPLUG_ENABLE (1 << 4)
-#define DIGITAL_PORTA_SHORT_PULSE_2MS (0 << 2)
-#define DIGITAL_PORTA_SHORT_PULSE_4_5MS (1 << 2)
-#define DIGITAL_PORTA_SHORT_PULSE_6MS (2 << 2)
-#define DIGITAL_PORTA_SHORT_PULSE_100MS (3 << 2)
-#define DIGITAL_PORTA_NO_DETECT (0 << 0)
-#define DIGITAL_PORTA_LONG_PULSE_DETECT_MASK (1 << 1)
-#define DIGITAL_PORTA_SHORT_PULSE_DETECT_MASK (1 << 0)
+#define DIGITAL_PORT_HOTPLUG_CNTRL 0x44030
+#define DIGITAL_PORTA_HOTPLUG_ENABLE (1 << 4)
+#define DIGITAL_PORTA_PULSE_DURATION_2ms (0 << 2)
+#define DIGITAL_PORTA_PULSE_DURATION_4_5ms (1 << 2)
+#define DIGITAL_PORTA_PULSE_DURATION_6ms (2 << 2)
+#define DIGITAL_PORTA_PULSE_DURATION_100ms (3 << 2)
+#define DIGITAL_PORTA_PULSE_DURATION_MASK (3 << 2)
+#define DIGITAL_PORTA_HOTPLUG_STATUS_MASK (3 << 0)
+#define DIGITAL_PORTA_HOTPLUG_NO_DETECT (0 << 0)
+#define DIGITAL_PORTA_HOTPLUG_SHORT_DETECT (1 << 0)
+#define DIGITAL_PORTA_HOTPLUG_LONG_DETECT (2 << 0)
/* refresh rate hardware control */
#define RR_HW_CTL 0x45300
@@ -6000,45 +6002,45 @@ enum skl_disp_power_wells {
/* digital port hotplug */
#define PCH_PORT_HOTPLUG 0xc4030 /* SHOTPLUG_CTL */
-#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
-#define BXT_PORTA_HOTPLUG_STATUS_MASK (0x3 << 24)
+#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
+#define BXT_PORTA_HOTPLUG_STATUS_MASK (3 << 24)
#define BXT_PORTA_HOTPLUG_NO_DETECT (0 << 24)
#define BXT_PORTA_HOTPLUG_SHORT_DETECT (1 << 24)
#define BXT_PORTA_HOTPLUG_LONG_DETECT (2 << 24)
-#define PORTD_HOTPLUG_ENABLE (1 << 20)
-#define PORTD_PULSE_DURATION_2ms (0)
-#define PORTD_PULSE_DURATION_4_5ms (1 << 18)
-#define PORTD_PULSE_DURATION_6ms (2 << 18)
-#define PORTD_PULSE_DURATION_100ms (3 << 18)
-#define PORTD_PULSE_DURATION_MASK (3 << 18)
-#define PORTD_HOTPLUG_STATUS_MASK (0x3 << 16)
+#define PORTD_HOTPLUG_ENABLE (1 << 20)
+#define PORTD_PULSE_DURATION_2ms (0 << 18)
+#define PORTD_PULSE_DURATION_4_5ms (1 << 18)
+#define PORTD_PULSE_DURATION_6ms (2 << 18)
+#define PORTD_PULSE_DURATION_100ms (3 << 18)
+#define PORTD_PULSE_DURATION_MASK (3 << 18)
+#define PORTD_HOTPLUG_STATUS_MASK (3 << 16)
#define PORTD_HOTPLUG_NO_DETECT (0 << 16)
#define PORTD_HOTPLUG_SHORT_DETECT (1 << 16)
#define PORTD_HOTPLUG_LONG_DETECT (2 << 16)
-#define PORTC_HOTPLUG_ENABLE (1 << 12)
-#define PORTC_PULSE_DURATION_2ms (0)
-#define PORTC_PULSE_DURATION_4_5ms (1 << 10)
-#define PORTC_PULSE_DURATION_6ms (2 << 10)
-#define PORTC_PULSE_DURATION_100ms (3 << 10)
-#define PORTC_PULSE_DURATION_MASK (3 << 10)
-#define PORTC_HOTPLUG_STATUS_MASK (0x3 << 8)
+#define PORTC_HOTPLUG_ENABLE (1 << 12)
+#define PORTC_PULSE_DURATION_2ms (0 << 10)
+#define PORTC_PULSE_DURATION_4_5ms (1 << 10)
+#define PORTC_PULSE_DURATION_6ms (2 << 10)
+#define PORTC_PULSE_DURATION_100ms (3 << 10)
+#define PORTC_PULSE_DURATION_MASK (3 << 10)
+#define PORTC_HOTPLUG_STATUS_MASK (3 << 8)
#define PORTC_HOTPLUG_NO_DETECT (0 << 8)
#define PORTC_HOTPLUG_SHORT_DETECT (1 << 8)
#define PORTC_HOTPLUG_LONG_DETECT (2 << 8)
-#define PORTB_HOTPLUG_ENABLE (1 << 4)
-#define PORTB_PULSE_DURATION_2ms (0)
-#define PORTB_PULSE_DURATION_4_5ms (1 << 2)
-#define PORTB_PULSE_DURATION_6ms (2 << 2)
-#define PORTB_PULSE_DURATION_100ms (3 << 2)
-#define PORTB_PULSE_DURATION_MASK (3 << 2)
-#define PORTB_HOTPLUG_STATUS_MASK (0x3 << 0)
+#define PORTB_HOTPLUG_ENABLE (1 << 4)
+#define PORTB_PULSE_DURATION_2ms (0 << 2)
+#define PORTB_PULSE_DURATION_4_5ms (1 << 2)
+#define PORTB_PULSE_DURATION_6ms (2 << 2)
+#define PORTB_PULSE_DURATION_100ms (3 << 2)
+#define PORTB_PULSE_DURATION_MASK (3 << 2)
+#define PORTB_HOTPLUG_STATUS_MASK (3 << 0)
#define PORTB_HOTPLUG_NO_DETECT (0 << 0)
#define PORTB_HOTPLUG_SHORT_DETECT (1 << 0)
#define PORTB_HOTPLUG_LONG_DETECT (2 << 0)
-#define PCH_PORT_HOTPLUG2 0xc403C /* SHOTPLUG_CTL2 */
-#define PORTE_HOTPLUG_ENABLE (1 << 4)
-#define PORTE_HOTPLUG_STATUS_MASK (0x3 << 0)
+#define PCH_PORT_HOTPLUG2 0xc403C /* SHOTPLUG_CTL2 SPT+ */
+#define PORTE_HOTPLUG_ENABLE (1 << 4)
+#define PORTE_HOTPLUG_STATUS_MASK (3 << 0)
#define PORTE_HOTPLUG_NO_DETECT (0 << 0)
#define PORTE_HOTPLUG_SHORT_DETECT (1 << 0)
#define PORTE_HOTPLUG_LONG_DETECT (2 << 0)
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 01/11] drm/i915: Clean up various HPD defines
2015-08-12 15:44 ` [PATCH 01/11] drm/i915: Clean up various HPD defines ville.syrjala
@ 2015-08-17 19:51 ` Paulo Zanoni
2015-08-26 18:23 ` Paulo Zanoni
0 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-17 19:51 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Indent the PORTx_HOTPLUG_... defines appropriately, and fix some space
> vs. tab issues.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 72 +++++++++++++++++++++--------------------
> 1 file changed, 37 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6786e94..ed2d150 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5364,15 +5364,17 @@ enum skl_disp_power_wells {
>
> #define CPU_VGACNTRL 0x41000
>
> -#define DIGITAL_PORT_HOTPLUG_CNTRL 0x44030
Maybe add a comment for the fields that are only valid up to IVB?
> -#define DIGITAL_PORTA_HOTPLUG_ENABLE (1 << 4)
> -#define DIGITAL_PORTA_SHORT_PULSE_2MS (0 << 2)
> -#define DIGITAL_PORTA_SHORT_PULSE_4_5MS (1 << 2)
> -#define DIGITAL_PORTA_SHORT_PULSE_6MS (2 << 2)
> -#define DIGITAL_PORTA_SHORT_PULSE_100MS (3 << 2)
> -#define DIGITAL_PORTA_NO_DETECT (0 << 0)
> -#define DIGITAL_PORTA_LONG_PULSE_DETECT_MASK (1 << 1)
> -#define DIGITAL_PORTA_SHORT_PULSE_DETECT_MASK (1 << 0)
> +#define DIGITAL_PORT_HOTPLUG_CNTRL 0x44030
> +#define DIGITAL_PORTA_HOTPLUG_ENABLE (1 << 4)
> +#define DIGITAL_PORTA_PULSE_DURATION_2ms (0 << 2)
I think I prefer the old SHORT_PULSE_duration names.
> +#define DIGITAL_PORTA_PULSE_DURATION_4_5ms (1 << 2)
> +#define DIGITAL_PORTA_PULSE_DURATION_6ms (2 << 2)
> +#define DIGITAL_PORTA_PULSE_DURATION_100ms (3 << 2)
> +#define DIGITAL_PORTA_PULSE_DURATION_MASK (3 << 2)
> +#define DIGITAL_PORTA_HOTPLUG_STATUS_MASK (3 << 0)
> +#define DIGITAL_PORTA_HOTPLUG_NO_DETECT (0 << 0)
> +#define DIGITAL_PORTA_HOTPLUG_SHORT_DETECT (1 << 0)
> +#define DIGITAL_PORTA_HOTPLUG_LONG_DETECT (2 << 0)
>
> /* refresh rate hardware control */
> #define RR_HW_CTL 0x45300
> @@ -6000,45 +6002,45 @@ enum skl_disp_power_wells {
>
> /* digital port hotplug */
> #define PCH_PORT_HOTPLUG 0xc4030 /* SHOTPLUG_CTL */
> -#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
> -#define BXT_PORTA_HOTPLUG_STATUS_MASK (0x3 << 24)
> +#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
> +#define BXT_PORTA_HOTPLUG_STATUS_MASK (3 << 24)
> #define BXT_PORTA_HOTPLUG_NO_DETECT (0 << 24)
> #define BXT_PORTA_HOTPLUG_SHORT_DETECT (1 << 24)
> #define BXT_PORTA_HOTPLUG_LONG_DETECT (2 << 24)
> -#define PORTD_HOTPLUG_ENABLE (1 << 20)
> -#define PORTD_PULSE_DURATION_2ms (0)
> -#define PORTD_PULSE_DURATION_4_5ms (1 << 18)
> -#define PORTD_PULSE_DURATION_6ms (2 << 18)
> -#define PORTD_PULSE_DURATION_100ms (3 << 18)
> -#define PORTD_PULSE_DURATION_MASK (3 << 18)
> -#define PORTD_HOTPLUG_STATUS_MASK (0x3 << 16)
> +#define PORTD_HOTPLUG_ENABLE (1 << 20)
> +#define PORTD_PULSE_DURATION_2ms (0 << 18)
> +#define PORTD_PULSE_DURATION_4_5ms (1 << 18)
> +#define PORTD_PULSE_DURATION_6ms (2 << 18)
> +#define PORTD_PULSE_DURATION_100ms (3 << 18)
> +#define PORTD_PULSE_DURATION_MASK (3 << 18)
> +#define PORTD_HOTPLUG_STATUS_MASK (3 << 16)
> #define PORTD_HOTPLUG_NO_DETECT (0 << 16)
> #define PORTD_HOTPLUG_SHORT_DETECT (1 << 16)
> #define PORTD_HOTPLUG_LONG_DETECT (2 << 16)
> -#define PORTC_HOTPLUG_ENABLE (1 << 12)
> -#define PORTC_PULSE_DURATION_2ms (0)
> -#define PORTC_PULSE_DURATION_4_5ms (1 << 10)
> -#define PORTC_PULSE_DURATION_6ms (2 << 10)
> -#define PORTC_PULSE_DURATION_100ms (3 << 10)
> -#define PORTC_PULSE_DURATION_MASK (3 << 10)
> -#define PORTC_HOTPLUG_STATUS_MASK (0x3 << 8)
> +#define PORTC_HOTPLUG_ENABLE (1 << 12)
> +#define PORTC_PULSE_DURATION_2ms (0 << 10)
> +#define PORTC_PULSE_DURATION_4_5ms (1 << 10)
> +#define PORTC_PULSE_DURATION_6ms (2 << 10)
> +#define PORTC_PULSE_DURATION_100ms (3 << 10)
> +#define PORTC_PULSE_DURATION_MASK (3 << 10)
> +#define PORTC_HOTPLUG_STATUS_MASK (3 << 8)
> #define PORTC_HOTPLUG_NO_DETECT (0 << 8)
> #define PORTC_HOTPLUG_SHORT_DETECT (1 << 8)
> #define PORTC_HOTPLUG_LONG_DETECT (2 << 8)
> -#define PORTB_HOTPLUG_ENABLE (1 << 4)
> -#define PORTB_PULSE_DURATION_2ms (0)
> -#define PORTB_PULSE_DURATION_4_5ms (1 << 2)
> -#define PORTB_PULSE_DURATION_6ms (2 << 2)
> -#define PORTB_PULSE_DURATION_100ms (3 << 2)
> -#define PORTB_PULSE_DURATION_MASK (3 << 2)
> -#define PORTB_HOTPLUG_STATUS_MASK (0x3 << 0)
> +#define PORTB_HOTPLUG_ENABLE (1 << 4)
> +#define PORTB_PULSE_DURATION_2ms (0 << 2)
> +#define PORTB_PULSE_DURATION_4_5ms (1 << 2)
> +#define PORTB_PULSE_DURATION_6ms (2 << 2)
> +#define PORTB_PULSE_DURATION_100ms (3 << 2)
> +#define PORTB_PULSE_DURATION_MASK (3 << 2)
> +#define PORTB_HOTPLUG_STATUS_MASK (3 << 0)
> #define PORTB_HOTPLUG_NO_DETECT (0 << 0)
> #define PORTB_HOTPLUG_SHORT_DETECT (1 << 0)
> #define PORTB_HOTPLUG_LONG_DETECT (2 << 0)
Maybe we could make something like "#define
HOTPLUG_PULSE_DURATION_MASK(port) (2 << (port) + X)".
>
> -#define PCH_PORT_HOTPLUG2 0xc403C /* SHOTPLUG_CTL2 */
> -#define PORTE_HOTPLUG_ENABLE (1 << 4)
> -#define PORTE_HOTPLUG_STATUS_MASK (0x3 << 0)
> +#define PCH_PORT_HOTPLUG2 0xc403C /* SHOTPLUG_CTL2 SPT+ */
> +#define PORTE_HOTPLUG_ENABLE (1 << 4)
> +#define PORTE_HOTPLUG_STATUS_MASK (3 << 0)
I was going to give the R-B despite the bikesheds, but this chunk
doesn't apply. The patch that adds the lines you're fixing here was
not merged yet. Maybe you could give your review comments to the
author while it's not merged :)
> #define PORTE_HOTPLUG_NO_DETECT (0 << 0)
> #define PORTE_HOTPLUG_SHORT_DETECT (1 << 0)
> #define PORTE_HOTPLUG_LONG_DETECT (2 << 0)
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 01/11] drm/i915: Clean up various HPD defines
2015-08-17 19:51 ` Paulo Zanoni
@ 2015-08-26 18:23 ` Paulo Zanoni
0 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-26 18:23 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-17 16:51 GMT-03:00 Paulo Zanoni <przanoni@gmail.com>:
> 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Indent the PORTx_HOTPLUG_... defines appropriately, and fix some space
>> vs. tab issues.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 72 +++++++++++++++++++++--------------------
>> 1 file changed, 37 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 6786e94..ed2d150 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5364,15 +5364,17 @@ enum skl_disp_power_wells {
>>
>> #define CPU_VGACNTRL 0x41000
>>
>> -#define DIGITAL_PORT_HOTPLUG_CNTRL 0x44030
>
> Maybe add a comment for the fields that are only valid up to IVB?
>
>> -#define DIGITAL_PORTA_HOTPLUG_ENABLE (1 << 4)
>> -#define DIGITAL_PORTA_SHORT_PULSE_2MS (0 << 2)
>> -#define DIGITAL_PORTA_SHORT_PULSE_4_5MS (1 << 2)
>> -#define DIGITAL_PORTA_SHORT_PULSE_6MS (2 << 2)
>> -#define DIGITAL_PORTA_SHORT_PULSE_100MS (3 << 2)
>> -#define DIGITAL_PORTA_NO_DETECT (0 << 0)
>> -#define DIGITAL_PORTA_LONG_PULSE_DETECT_MASK (1 << 1)
>> -#define DIGITAL_PORTA_SHORT_PULSE_DETECT_MASK (1 << 0)
>> +#define DIGITAL_PORT_HOTPLUG_CNTRL 0x44030
>> +#define DIGITAL_PORTA_HOTPLUG_ENABLE (1 << 4)
>> +#define DIGITAL_PORTA_PULSE_DURATION_2ms (0 << 2)
>
> I think I prefer the old SHORT_PULSE_duration names.
>
>> +#define DIGITAL_PORTA_PULSE_DURATION_4_5ms (1 << 2)
>> +#define DIGITAL_PORTA_PULSE_DURATION_6ms (2 << 2)
>> +#define DIGITAL_PORTA_PULSE_DURATION_100ms (3 << 2)
>> +#define DIGITAL_PORTA_PULSE_DURATION_MASK (3 << 2)
>> +#define DIGITAL_PORTA_HOTPLUG_STATUS_MASK (3 << 0)
>> +#define DIGITAL_PORTA_HOTPLUG_NO_DETECT (0 << 0)
>> +#define DIGITAL_PORTA_HOTPLUG_SHORT_DETECT (1 << 0)
>> +#define DIGITAL_PORTA_HOTPLUG_LONG_DETECT (2 << 0)
>>
>> /* refresh rate hardware control */
>> #define RR_HW_CTL 0x45300
>> @@ -6000,45 +6002,45 @@ enum skl_disp_power_wells {
>>
>> /* digital port hotplug */
>> #define PCH_PORT_HOTPLUG 0xc4030 /* SHOTPLUG_CTL */
Another bikeshed: use tabs between the name and the reg address on the
line above?
>> -#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
>> -#define BXT_PORTA_HOTPLUG_STATUS_MASK (0x3 << 24)
>> +#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
>> +#define BXT_PORTA_HOTPLUG_STATUS_MASK (3 << 24)
>> #define BXT_PORTA_HOTPLUG_NO_DETECT (0 << 24)
>> #define BXT_PORTA_HOTPLUG_SHORT_DETECT (1 << 24)
>> #define BXT_PORTA_HOTPLUG_LONG_DETECT (2 << 24)
>> -#define PORTD_HOTPLUG_ENABLE (1 << 20)
>> -#define PORTD_PULSE_DURATION_2ms (0)
>> -#define PORTD_PULSE_DURATION_4_5ms (1 << 18)
>> -#define PORTD_PULSE_DURATION_6ms (2 << 18)
>> -#define PORTD_PULSE_DURATION_100ms (3 << 18)
>> -#define PORTD_PULSE_DURATION_MASK (3 << 18)
>> -#define PORTD_HOTPLUG_STATUS_MASK (0x3 << 16)
>> +#define PORTD_HOTPLUG_ENABLE (1 << 20)
>> +#define PORTD_PULSE_DURATION_2ms (0 << 18)
>> +#define PORTD_PULSE_DURATION_4_5ms (1 << 18)
>> +#define PORTD_PULSE_DURATION_6ms (2 << 18)
>> +#define PORTD_PULSE_DURATION_100ms (3 << 18)
>> +#define PORTD_PULSE_DURATION_MASK (3 << 18)
>> +#define PORTD_HOTPLUG_STATUS_MASK (3 << 16)
>> #define PORTD_HOTPLUG_NO_DETECT (0 << 16)
>> #define PORTD_HOTPLUG_SHORT_DETECT (1 << 16)
>> #define PORTD_HOTPLUG_LONG_DETECT (2 << 16)
>> -#define PORTC_HOTPLUG_ENABLE (1 << 12)
>> -#define PORTC_PULSE_DURATION_2ms (0)
>> -#define PORTC_PULSE_DURATION_4_5ms (1 << 10)
>> -#define PORTC_PULSE_DURATION_6ms (2 << 10)
>> -#define PORTC_PULSE_DURATION_100ms (3 << 10)
>> -#define PORTC_PULSE_DURATION_MASK (3 << 10)
>> -#define PORTC_HOTPLUG_STATUS_MASK (0x3 << 8)
>> +#define PORTC_HOTPLUG_ENABLE (1 << 12)
>> +#define PORTC_PULSE_DURATION_2ms (0 << 10)
>> +#define PORTC_PULSE_DURATION_4_5ms (1 << 10)
>> +#define PORTC_PULSE_DURATION_6ms (2 << 10)
>> +#define PORTC_PULSE_DURATION_100ms (3 << 10)
>> +#define PORTC_PULSE_DURATION_MASK (3 << 10)
>> +#define PORTC_HOTPLUG_STATUS_MASK (3 << 8)
>> #define PORTC_HOTPLUG_NO_DETECT (0 << 8)
>> #define PORTC_HOTPLUG_SHORT_DETECT (1 << 8)
>> #define PORTC_HOTPLUG_LONG_DETECT (2 << 8)
>> -#define PORTB_HOTPLUG_ENABLE (1 << 4)
>> -#define PORTB_PULSE_DURATION_2ms (0)
>> -#define PORTB_PULSE_DURATION_4_5ms (1 << 2)
>> -#define PORTB_PULSE_DURATION_6ms (2 << 2)
>> -#define PORTB_PULSE_DURATION_100ms (3 << 2)
>> -#define PORTB_PULSE_DURATION_MASK (3 << 2)
>> -#define PORTB_HOTPLUG_STATUS_MASK (0x3 << 0)
>> +#define PORTB_HOTPLUG_ENABLE (1 << 4)
>> +#define PORTB_PULSE_DURATION_2ms (0 << 2)
>> +#define PORTB_PULSE_DURATION_4_5ms (1 << 2)
>> +#define PORTB_PULSE_DURATION_6ms (2 << 2)
>> +#define PORTB_PULSE_DURATION_100ms (3 << 2)
>> +#define PORTB_PULSE_DURATION_MASK (3 << 2)
>> +#define PORTB_HOTPLUG_STATUS_MASK (3 << 0)
>> #define PORTB_HOTPLUG_NO_DETECT (0 << 0)
>> #define PORTB_HOTPLUG_SHORT_DETECT (1 << 0)
>> #define PORTB_HOTPLUG_LONG_DETECT (2 << 0)
>
> Maybe we could make something like "#define
> HOTPLUG_PULSE_DURATION_MASK(port) (2 << (port) + X)".
>
>>
>> -#define PCH_PORT_HOTPLUG2 0xc403C /* SHOTPLUG_CTL2 */
>> -#define PORTE_HOTPLUG_ENABLE (1 << 4)
>> -#define PORTE_HOTPLUG_STATUS_MASK (0x3 << 0)
>> +#define PCH_PORT_HOTPLUG2 0xc403C /* SHOTPLUG_CTL2 SPT+ */
Same for the line above.
>> +#define PORTE_HOTPLUG_ENABLE (1 << 4)
>> +#define PORTE_HOTPLUG_STATUS_MASK (3 << 0)
>
> I was going to give the R-B despite the bikesheds, but this chunk
> doesn't apply. The patch that adds the lines you're fixing here was
> not merged yet. Maybe you could give your review comments to the
> author while it's not merged :)
This now applies to -nightly due to the requirement patch being merged
to drm-intel-next-fixes. So it's a matter of waiting for the
backmerge.
With or without the bikesheds: Reviewed-by: Paulo Zanoni
<paulo.r.zanoni@intel.com>
>
>
>> #define PORTE_HOTPLUG_NO_DETECT (0 << 0)
>> #define PORTE_HOTPLUG_SHORT_DETECT (1 << 0)
>> #define PORTE_HOTPLUG_LONG_DETECT (2 << 0)
I wouldn't have complained if you had fixed the GPIO lines immediately
below these :)
>> --
>> 2.4.6
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 02/11] drm/i915; Extract intel_hpd_enabled_irqs()
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
2015-08-12 15:44 ` [PATCH 01/11] drm/i915: Clean up various HPD defines ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-17 20:06 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 03/11] drm/i915: Factor out ilk_update_display_irq() ville.syrjala
` (10 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Eliminate a bunch of duplicated code that calculates the currently
enabled HPD interrupt bits.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 43 ++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8485bea..de0edbd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3002,27 +3002,34 @@ static void cherryview_irq_preinstall(struct drm_device *dev)
vlv_display_irq_reset(dev_priv);
}
+static u32 intel_hpd_enabled_irqs(struct drm_device *dev,
+ const u32 hpd[HPD_NUM_PINS])
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct intel_encoder *encoder;
+ u32 enabled_irqs = 0;
+
+ for_each_intel_encoder(dev, encoder)
+ if (dev_priv->hotplug.stats[encoder->hpd_pin].state == HPD_ENABLED)
+ enabled_irqs |= hpd[encoder->hpd_pin];
+
+ return enabled_irqs;
+}
+
static void ibx_hpd_irq_setup(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_encoder *intel_encoder;
- u32 hotplug_irqs, hotplug, enabled_irqs = 0;
+ u32 hotplug_irqs, hotplug, enabled_irqs;
if (HAS_PCH_IBX(dev)) {
hotplug_irqs = SDE_HOTPLUG_MASK;
- for_each_intel_encoder(dev, intel_encoder)
- if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
- enabled_irqs |= hpd_ibx[intel_encoder->hpd_pin];
+ enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ibx);
} else if (HAS_PCH_SPT(dev)) {
hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
- for_each_intel_encoder(dev, intel_encoder)
- if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
- enabled_irqs |= hpd_spt[intel_encoder->hpd_pin];
+ enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
} else {
hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
- for_each_intel_encoder(dev, intel_encoder)
- if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
- enabled_irqs |= hpd_cpt[intel_encoder->hpd_pin];
+ enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_cpt);
}
ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
@@ -3051,15 +3058,10 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
static void bxt_hpd_irq_setup(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_encoder *intel_encoder;
- u32 hotplug_port = 0;
+ u32 hotplug_port;
u32 hotplug_ctrl;
- for_each_intel_encoder(dev, intel_encoder) {
- if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state
- == HPD_ENABLED)
- hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
- }
+ hotplug_port = intel_hpd_enabled_irqs(dev, hpd_bxt);
hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
@@ -3935,7 +3937,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
static void i915_hpd_irq_setup(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_encoder *intel_encoder;
u32 hotplug_en;
assert_spin_locked(&dev_priv->irq_lock);
@@ -3944,9 +3945,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
hotplug_en &= ~HOTPLUG_INT_EN_MASK;
/* Note HDMI and DP share hotplug bits */
/* enable bits are the same for all generations */
- for_each_intel_encoder(dev, intel_encoder)
- if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
- hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
+ hotplug_en |= intel_hpd_enabled_irqs(dev, hpd_mask_i915);
/* Programming the CRT detection parameters tends
to generate a spurious hotplug event about three
seconds later. So just do it once.
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/i915; Extract intel_hpd_enabled_irqs()
2015-08-12 15:44 ` [PATCH 02/11] drm/i915; Extract intel_hpd_enabled_irqs() ville.syrjala
@ 2015-08-17 20:06 ` Paulo Zanoni
2015-08-19 17:02 ` Ville Syrjälä
2015-08-26 18:30 ` Paulo Zanoni
0 siblings, 2 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-17 20:06 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Eliminate a bunch of duplicated code that calculates the currently
> enabled HPD interrupt bits.
Nice one! I see this one also depends on a patch that's not merged
yet, so I'm not sure if I should wait for it to be merged before
continuing the review, or if you plan to send a version rebased just
on -nightly.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 43 ++++++++++++++++++++---------------------
> 1 file changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 8485bea..de0edbd 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3002,27 +3002,34 @@ static void cherryview_irq_preinstall(struct drm_device *dev)
> vlv_display_irq_reset(dev_priv);
> }
>
> +static u32 intel_hpd_enabled_irqs(struct drm_device *dev,
> + const u32 hpd[HPD_NUM_PINS])
> +{
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_encoder *encoder;
> + u32 enabled_irqs = 0;
> +
> + for_each_intel_encoder(dev, encoder)
> + if (dev_priv->hotplug.stats[encoder->hpd_pin].state == HPD_ENABLED)
> + enabled_irqs |= hpd[encoder->hpd_pin];
> +
> + return enabled_irqs;
> +}
> +
> static void ibx_hpd_irq_setup(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_encoder *intel_encoder;
> - u32 hotplug_irqs, hotplug, enabled_irqs = 0;
> + u32 hotplug_irqs, hotplug, enabled_irqs;
>
> if (HAS_PCH_IBX(dev)) {
> hotplug_irqs = SDE_HOTPLUG_MASK;
> - for_each_intel_encoder(dev, intel_encoder)
> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> - enabled_irqs |= hpd_ibx[intel_encoder->hpd_pin];
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ibx);
> } else if (HAS_PCH_SPT(dev)) {
> hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
> - for_each_intel_encoder(dev, intel_encoder)
> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> - enabled_irqs |= hpd_spt[intel_encoder->hpd_pin];
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
> } else {
> hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
> - for_each_intel_encoder(dev, intel_encoder)
> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> - enabled_irqs |= hpd_cpt[intel_encoder->hpd_pin];
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_cpt);
> }
>
> ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
> @@ -3051,15 +3058,10 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
> static void bxt_hpd_irq_setup(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_encoder *intel_encoder;
> - u32 hotplug_port = 0;
> + u32 hotplug_port;
> u32 hotplug_ctrl;
>
> - for_each_intel_encoder(dev, intel_encoder) {
> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state
> - == HPD_ENABLED)
> - hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
> - }
> + hotplug_port = intel_hpd_enabled_irqs(dev, hpd_bxt);
>
> hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
>
> @@ -3935,7 +3937,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
> static void i915_hpd_irq_setup(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_encoder *intel_encoder;
> u32 hotplug_en;
>
> assert_spin_locked(&dev_priv->irq_lock);
> @@ -3944,9 +3945,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
> hotplug_en &= ~HOTPLUG_INT_EN_MASK;
> /* Note HDMI and DP share hotplug bits */
> /* enable bits are the same for all generations */
> - for_each_intel_encoder(dev, intel_encoder)
> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> - hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
> + hotplug_en |= intel_hpd_enabled_irqs(dev, hpd_mask_i915);
> /* Programming the CRT detection parameters tends
> to generate a spurious hotplug event about three
> seconds later. So just do it once.
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/i915; Extract intel_hpd_enabled_irqs()
2015-08-17 20:06 ` Paulo Zanoni
@ 2015-08-19 17:02 ` Ville Syrjälä
2015-08-26 18:30 ` Paulo Zanoni
1 sibling, 0 replies; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-19 17:02 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development
On Mon, Aug 17, 2015 at 05:06:17PM -0300, Paulo Zanoni wrote:
> 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Eliminate a bunch of duplicated code that calculates the currently
> > enabled HPD interrupt bits.
>
> Nice one! I see this one also depends on a patch that's not merged
> yet, so I'm not sure if I should wait for it to be merged before
> continuing the review, or if you plan to send a version rebased just
> on -nightly.
I rebased it on top of nightly before I sent it. I guess some patches
got dropped from nightly.
>
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 43 ++++++++++++++++++++---------------------
> > 1 file changed, 21 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 8485bea..de0edbd 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3002,27 +3002,34 @@ static void cherryview_irq_preinstall(struct drm_device *dev)
> > vlv_display_irq_reset(dev_priv);
> > }
> >
> > +static u32 intel_hpd_enabled_irqs(struct drm_device *dev,
> > + const u32 hpd[HPD_NUM_PINS])
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(dev);
> > + struct intel_encoder *encoder;
> > + u32 enabled_irqs = 0;
> > +
> > + for_each_intel_encoder(dev, encoder)
> > + if (dev_priv->hotplug.stats[encoder->hpd_pin].state == HPD_ENABLED)
> > + enabled_irqs |= hpd[encoder->hpd_pin];
> > +
> > + return enabled_irqs;
> > +}
> > +
> > static void ibx_hpd_irq_setup(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - struct intel_encoder *intel_encoder;
> > - u32 hotplug_irqs, hotplug, enabled_irqs = 0;
> > + u32 hotplug_irqs, hotplug, enabled_irqs;
> >
> > if (HAS_PCH_IBX(dev)) {
> > hotplug_irqs = SDE_HOTPLUG_MASK;
> > - for_each_intel_encoder(dev, intel_encoder)
> > - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> > - enabled_irqs |= hpd_ibx[intel_encoder->hpd_pin];
> > + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ibx);
> > } else if (HAS_PCH_SPT(dev)) {
> > hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
> > - for_each_intel_encoder(dev, intel_encoder)
> > - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> > - enabled_irqs |= hpd_spt[intel_encoder->hpd_pin];
> > + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
> > } else {
> > hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
> > - for_each_intel_encoder(dev, intel_encoder)
> > - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> > - enabled_irqs |= hpd_cpt[intel_encoder->hpd_pin];
> > + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_cpt);
> > }
> >
> > ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
> > @@ -3051,15 +3058,10 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
> > static void bxt_hpd_irq_setup(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - struct intel_encoder *intel_encoder;
> > - u32 hotplug_port = 0;
> > + u32 hotplug_port;
> > u32 hotplug_ctrl;
> >
> > - for_each_intel_encoder(dev, intel_encoder) {
> > - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state
> > - == HPD_ENABLED)
> > - hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
> > - }
> > + hotplug_port = intel_hpd_enabled_irqs(dev, hpd_bxt);
> >
> > hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
> >
> > @@ -3935,7 +3937,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
> > static void i915_hpd_irq_setup(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - struct intel_encoder *intel_encoder;
> > u32 hotplug_en;
> >
> > assert_spin_locked(&dev_priv->irq_lock);
> > @@ -3944,9 +3945,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
> > hotplug_en &= ~HOTPLUG_INT_EN_MASK;
> > /* Note HDMI and DP share hotplug bits */
> > /* enable bits are the same for all generations */
> > - for_each_intel_encoder(dev, intel_encoder)
> > - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
> > - hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
> > + hotplug_en |= intel_hpd_enabled_irqs(dev, hpd_mask_i915);
> > /* Programming the CRT detection parameters tends
> > to generate a spurious hotplug event about three
> > seconds later. So just do it once.
> > --
> > 2.4.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/i915; Extract intel_hpd_enabled_irqs()
2015-08-17 20:06 ` Paulo Zanoni
2015-08-19 17:02 ` Ville Syrjälä
@ 2015-08-26 18:30 ` Paulo Zanoni
1 sibling, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-26 18:30 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-17 17:06 GMT-03:00 Paulo Zanoni <przanoni@gmail.com>:
> 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Eliminate a bunch of duplicated code that calculates the currently
>> enabled HPD interrupt bits.
>
> Nice one! I see this one also depends on a patch that's not merged
> yet, so I'm not sure if I should wait for it to be merged before
> continuing the review, or if you plan to send a version rebased just
> on -nightly.
This one is also unblocked, same reason as patch 1.
The bikeshed goes to the patch title: s/i915; Extract/i915: Extract/ .
I guess Daniel can apply this one while merging.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_irq.c | 43 ++++++++++++++++++++---------------------
>> 1 file changed, 21 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 8485bea..de0edbd 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -3002,27 +3002,34 @@ static void cherryview_irq_preinstall(struct drm_device *dev)
>> vlv_display_irq_reset(dev_priv);
>> }
>>
>> +static u32 intel_hpd_enabled_irqs(struct drm_device *dev,
>> + const u32 hpd[HPD_NUM_PINS])
>> +{
>> + struct drm_i915_private *dev_priv = to_i915(dev);
>> + struct intel_encoder *encoder;
>> + u32 enabled_irqs = 0;
>> +
>> + for_each_intel_encoder(dev, encoder)
>> + if (dev_priv->hotplug.stats[encoder->hpd_pin].state == HPD_ENABLED)
>> + enabled_irqs |= hpd[encoder->hpd_pin];
>> +
>> + return enabled_irqs;
>> +}
>> +
>> static void ibx_hpd_irq_setup(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> - struct intel_encoder *intel_encoder;
>> - u32 hotplug_irqs, hotplug, enabled_irqs = 0;
>> + u32 hotplug_irqs, hotplug, enabled_irqs;
>>
>> if (HAS_PCH_IBX(dev)) {
>> hotplug_irqs = SDE_HOTPLUG_MASK;
>> - for_each_intel_encoder(dev, intel_encoder)
>> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
>> - enabled_irqs |= hpd_ibx[intel_encoder->hpd_pin];
>> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ibx);
>> } else if (HAS_PCH_SPT(dev)) {
>> hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
>> - for_each_intel_encoder(dev, intel_encoder)
>> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
>> - enabled_irqs |= hpd_spt[intel_encoder->hpd_pin];
>> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
>> } else {
>> hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
>> - for_each_intel_encoder(dev, intel_encoder)
>> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
>> - enabled_irqs |= hpd_cpt[intel_encoder->hpd_pin];
>> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_cpt);
>> }
>>
>> ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
>> @@ -3051,15 +3058,10 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
>> static void bxt_hpd_irq_setup(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> - struct intel_encoder *intel_encoder;
>> - u32 hotplug_port = 0;
>> + u32 hotplug_port;
>> u32 hotplug_ctrl;
>>
>> - for_each_intel_encoder(dev, intel_encoder) {
>> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state
>> - == HPD_ENABLED)
>> - hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
>> - }
>> + hotplug_port = intel_hpd_enabled_irqs(dev, hpd_bxt);
>>
>> hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
>>
>> @@ -3935,7 +3937,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
>> static void i915_hpd_irq_setup(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> - struct intel_encoder *intel_encoder;
>> u32 hotplug_en;
>>
>> assert_spin_locked(&dev_priv->irq_lock);
>> @@ -3944,9 +3945,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
>> hotplug_en &= ~HOTPLUG_INT_EN_MASK;
>> /* Note HDMI and DP share hotplug bits */
>> /* enable bits are the same for all generations */
>> - for_each_intel_encoder(dev, intel_encoder)
>> - if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state == HPD_ENABLED)
>> - hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
>> + hotplug_en |= intel_hpd_enabled_irqs(dev, hpd_mask_i915);
>> /* Programming the CRT detection parameters tends
>> to generate a spurious hotplug event about three
>> seconds later. So just do it once.
>> --
>> 2.4.6
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 03/11] drm/i915: Factor out ilk_update_display_irq()
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
2015-08-12 15:44 ` [PATCH 01/11] drm/i915: Clean up various HPD defines ville.syrjala
2015-08-12 15:44 ` [PATCH 02/11] drm/i915; Extract intel_hpd_enabled_irqs() ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-26 18:46 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 04/11] drm/i915: Add HAS_PCH_LPT_LP() macro ville.syrjala
` (9 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Extract the core of ironlake_{enable,disable}_display_irq() into a new
function. We'll have further use for it later.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 39 ++++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index de0edbd..8a1e35e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -154,35 +154,44 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
-/* For display hotplug interrupt */
-void
-ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
+/**
+ * ilk_update_display_irq - update DEIMR
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ */
+static void ilk_update_display_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
{
+ uint32_t new_val;
+
assert_spin_locked(&dev_priv->irq_lock);
if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return;
- if ((dev_priv->irq_mask & mask) != 0) {
- dev_priv->irq_mask &= ~mask;
+ new_val = dev_priv->irq_mask;
+ new_val &= ~interrupt_mask;
+ new_val |= (~enabled_irq_mask & interrupt_mask);
+
+ if (new_val != dev_priv->irq_mask) {
+ dev_priv->irq_mask = new_val;
I915_WRITE(DEIMR, dev_priv->irq_mask);
POSTING_READ(DEIMR);
}
}
void
-ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
+ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
{
- assert_spin_locked(&dev_priv->irq_lock);
-
- if (WARN_ON(!intel_irqs_enabled(dev_priv)))
- return;
+ ilk_update_display_irq(dev_priv, mask, mask);
+}
- if ((dev_priv->irq_mask & mask) != mask) {
- dev_priv->irq_mask |= mask;
- I915_WRITE(DEIMR, dev_priv->irq_mask);
- POSTING_READ(DEIMR);
- }
+void
+ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
+{
+ ilk_update_display_irq(dev_priv, mask, 0);
}
/**
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 03/11] drm/i915: Factor out ilk_update_display_irq()
2015-08-12 15:44 ` [PATCH 03/11] drm/i915: Factor out ilk_update_display_irq() ville.syrjala
@ 2015-08-26 18:46 ` Paulo Zanoni
0 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-26 18:46 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Extract the core of ironlake_{enable,disable}_display_irq() into a new
> function. We'll have further use for it later.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 39 ++++++++++++++++++++++++---------------
> 1 file changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index de0edbd..8a1e35e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -154,35 +154,44 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
>
> static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
>
> -/* For display hotplug interrupt */
> -void
> -ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
> +/**
> + * ilk_update_display_irq - update DEIMR
> + * @dev_priv: driver private
> + * @interrupt_mask: mask of interrupt bits to update
> + * @enabled_irq_mask: mask of interrupt bits to enable
> + */
> +static void ilk_update_display_irq(struct drm_i915_private *dev_priv,
> + uint32_t interrupt_mask,
> + uint32_t enabled_irq_mask)
> {
> + uint32_t new_val;
> +
> assert_spin_locked(&dev_priv->irq_lock);
>
WARN_ON(enabled_irq_mask & ~interrupt_mask);
With that: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> return;
>
> - if ((dev_priv->irq_mask & mask) != 0) {
> - dev_priv->irq_mask &= ~mask;
> + new_val = dev_priv->irq_mask;
> + new_val &= ~interrupt_mask;
> + new_val |= (~enabled_irq_mask & interrupt_mask);
> +
> + if (new_val != dev_priv->irq_mask) {
> + dev_priv->irq_mask = new_val;
> I915_WRITE(DEIMR, dev_priv->irq_mask);
> POSTING_READ(DEIMR);
> }
> }
>
> void
> -ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
> +ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
> {
> - assert_spin_locked(&dev_priv->irq_lock);
> -
> - if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> - return;
> + ilk_update_display_irq(dev_priv, mask, mask);
> +}
>
> - if ((dev_priv->irq_mask & mask) != mask) {
> - dev_priv->irq_mask |= mask;
> - I915_WRITE(DEIMR, dev_priv->irq_mask);
> - POSTING_READ(DEIMR);
> - }
> +void
> +ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
> +{
> + ilk_update_display_irq(dev_priv, mask, 0);
> }
>
> /**
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 04/11] drm/i915: Add HAS_PCH_LPT_LP() macro
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (2 preceding siblings ...)
2015-08-12 15:44 ` [PATCH 03/11] drm/i915: Factor out ilk_update_display_irq() ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-26 18:58 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines ville.syrjala
` (8 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Make LPT:LP checks look neater by wrapping the details in a
new HAS_PCH_LPT_LP() macro.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 13 +++++--------
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 55611d8..4e391dd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2573,6 +2573,7 @@ struct drm_i915_cmd_table {
#define INTEL_PCH_TYPE(dev) (__I915__(dev)->pch_type)
#define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT)
#define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT)
+#define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
#define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
#define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
#define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4b3012b..97c6368 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8381,8 +8381,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread,
if (WARN(with_fdi && !with_spread, "FDI requires downspread\n"))
with_spread = true;
- if (WARN(dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE &&
- with_fdi, "LP PCH doesn't have FDI\n"))
+ if (WARN(HAS_PCH_LPT_LP(dev) && with_fdi, "LP PCH doesn't have FDI\n"))
with_fdi = false;
mutex_lock(&dev_priv->sb_lock);
@@ -8405,8 +8404,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread,
}
}
- reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
- SBI_GEN0 : SBI_DBUFF0;
+ reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0;
tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
tmp |= SBI_GEN0_CFG_BUFFENABLE_DISABLE;
intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
@@ -8422,8 +8420,7 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
mutex_lock(&dev_priv->sb_lock);
- reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
- SBI_GEN0 : SBI_DBUFF0;
+ reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0;
tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
tmp &= ~SBI_GEN0_CFG_BUFFENABLE_DISABLE;
intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
@@ -9435,7 +9432,7 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv)
DRM_DEBUG_KMS("Enabling package C8+\n");
- if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
+ if (HAS_PCH_LPT_LP(dev)) {
val = I915_READ(SOUTH_DSPCLK_GATE_D);
val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
@@ -9455,7 +9452,7 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
hsw_restore_lcpll(dev_priv);
lpt_init_pch_refclk(dev);
- if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
+ if (HAS_PCH_LPT_LP(dev)) {
val = I915_READ(SOUTH_DSPCLK_GATE_D);
val |= PCH_LP_PARTITION_LEVEL_DISABLE;
I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fff0c226..ea49661 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6588,7 +6588,7 @@ static void lpt_init_clock_gating(struct drm_device *dev)
* TODO: this bit should only be enabled when really needed, then
* disabled when not needed anymore in order to save power.
*/
- if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
+ if (HAS_PCH_LPT_LP(dev))
I915_WRITE(SOUTH_DSPCLK_GATE_D,
I915_READ(SOUTH_DSPCLK_GATE_D) |
PCH_LP_PARTITION_LEVEL_DISABLE);
@@ -6603,7 +6603,7 @@ static void lpt_suspend_hw(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
+ if (HAS_PCH_LPT_LP(dev)) {
uint32_t val = I915_READ(SOUTH_DSPCLK_GATE_D);
val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 04/11] drm/i915: Add HAS_PCH_LPT_LP() macro
2015-08-12 15:44 ` [PATCH 04/11] drm/i915: Add HAS_PCH_LPT_LP() macro ville.syrjala
@ 2015-08-26 18:58 ` Paulo Zanoni
2015-08-27 16:32 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-26 18:58 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make LPT:LP checks look neater by wrapping the details in a
> new HAS_PCH_LPT_LP() macro.
This has the potential to be confusing since HAS_PCH_LPT() is also
true for cases where HAS_PCH_LPT_LP() is true. Maybe we could rename
the macro in some way that it wouldn't be HAS_PCH_XXX in order to
reduce the probability of confusion? But I can't think of any
suggestions...
Anyway, the patch is technically correct, so I'll let the decision to
you and Daniel.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_display.c | 13 +++++--------
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> 3 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 55611d8..4e391dd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2573,6 +2573,7 @@ struct drm_i915_cmd_table {
> #define INTEL_PCH_TYPE(dev) (__I915__(dev)->pch_type)
> #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT)
> #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT)
> +#define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
> #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
> #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
> #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4b3012b..97c6368 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8381,8 +8381,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread,
>
> if (WARN(with_fdi && !with_spread, "FDI requires downspread\n"))
> with_spread = true;
> - if (WARN(dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE &&
> - with_fdi, "LP PCH doesn't have FDI\n"))
> + if (WARN(HAS_PCH_LPT_LP(dev) && with_fdi, "LP PCH doesn't have FDI\n"))
> with_fdi = false;
>
> mutex_lock(&dev_priv->sb_lock);
> @@ -8405,8 +8404,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread,
> }
> }
>
> - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
> - SBI_GEN0 : SBI_DBUFF0;
> + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0;
> tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
> tmp |= SBI_GEN0_CFG_BUFFENABLE_DISABLE;
> intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
> @@ -8422,8 +8420,7 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
>
> mutex_lock(&dev_priv->sb_lock);
>
> - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
> - SBI_GEN0 : SBI_DBUFF0;
> + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0;
> tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
> tmp &= ~SBI_GEN0_CFG_BUFFENABLE_DISABLE;
> intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
> @@ -9435,7 +9432,7 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv)
>
> DRM_DEBUG_KMS("Enabling package C8+\n");
>
> - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
> + if (HAS_PCH_LPT_LP(dev)) {
> val = I915_READ(SOUTH_DSPCLK_GATE_D);
> val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
> I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
> @@ -9455,7 +9452,7 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
> hsw_restore_lcpll(dev_priv);
> lpt_init_pch_refclk(dev);
>
> - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
> + if (HAS_PCH_LPT_LP(dev)) {
> val = I915_READ(SOUTH_DSPCLK_GATE_D);
> val |= PCH_LP_PARTITION_LEVEL_DISABLE;
> I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fff0c226..ea49661 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6588,7 +6588,7 @@ static void lpt_init_clock_gating(struct drm_device *dev)
> * TODO: this bit should only be enabled when really needed, then
> * disabled when not needed anymore in order to save power.
> */
> - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
> + if (HAS_PCH_LPT_LP(dev))
> I915_WRITE(SOUTH_DSPCLK_GATE_D,
> I915_READ(SOUTH_DSPCLK_GATE_D) |
> PCH_LP_PARTITION_LEVEL_DISABLE);
> @@ -6603,7 +6603,7 @@ static void lpt_suspend_hw(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
> + if (HAS_PCH_LPT_LP(dev)) {
> uint32_t val = I915_READ(SOUTH_DSPCLK_GATE_D);
>
> val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 04/11] drm/i915: Add HAS_PCH_LPT_LP() macro
2015-08-26 18:58 ` Paulo Zanoni
@ 2015-08-27 16:32 ` Ville Syrjälä
0 siblings, 0 replies; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-27 16:32 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development
On Wed, Aug 26, 2015 at 03:58:11PM -0300, Paulo Zanoni wrote:
> 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make LPT:LP checks look neater by wrapping the details in a
> > new HAS_PCH_LPT_LP() macro.
>
> This has the potential to be confusing since HAS_PCH_LPT() is also
> true for cases where HAS_PCH_LPT_LP() is true. Maybe we could rename
> the macro in some way that it wouldn't be HAS_PCH_XXX in order to
> reduce the probability of confusion? But I can't think of any
> suggestions...
We could rename HAS_PCH_LPT to HAS_PCH_LPT_LP_OR_H but that's just too
ugly IMO. I guess it might be clearer if we also had HAS_PCH_LPT_H but
since we have not use for both adding one is a bit dubious. So no nice
ideas here either.
>
> Anyway, the patch is technically correct, so I'll let the decision to
> you and Daniel.
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_display.c | 13 +++++--------
> > drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> > 3 files changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 55611d8..4e391dd 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2573,6 +2573,7 @@ struct drm_i915_cmd_table {
> > #define INTEL_PCH_TYPE(dev) (__I915__(dev)->pch_type)
> > #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT)
> > #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT)
> > +#define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
> > #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
> > #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
> > #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 4b3012b..97c6368 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8381,8 +8381,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread,
> >
> > if (WARN(with_fdi && !with_spread, "FDI requires downspread\n"))
> > with_spread = true;
> > - if (WARN(dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE &&
> > - with_fdi, "LP PCH doesn't have FDI\n"))
> > + if (WARN(HAS_PCH_LPT_LP(dev) && with_fdi, "LP PCH doesn't have FDI\n"))
> > with_fdi = false;
> >
> > mutex_lock(&dev_priv->sb_lock);
> > @@ -8405,8 +8404,7 @@ static void lpt_enable_clkout_dp(struct drm_device *dev, bool with_spread,
> > }
> > }
> >
> > - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
> > - SBI_GEN0 : SBI_DBUFF0;
> > + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0;
> > tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
> > tmp |= SBI_GEN0_CFG_BUFFENABLE_DISABLE;
> > intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
> > @@ -8422,8 +8420,7 @@ static void lpt_disable_clkout_dp(struct drm_device *dev)
> >
> > mutex_lock(&dev_priv->sb_lock);
> >
> > - reg = (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) ?
> > - SBI_GEN0 : SBI_DBUFF0;
> > + reg = HAS_PCH_LPT_LP(dev) ? SBI_GEN0 : SBI_DBUFF0;
> > tmp = intel_sbi_read(dev_priv, reg, SBI_ICLK);
> > tmp &= ~SBI_GEN0_CFG_BUFFENABLE_DISABLE;
> > intel_sbi_write(dev_priv, reg, tmp, SBI_ICLK);
> > @@ -9435,7 +9432,7 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv)
> >
> > DRM_DEBUG_KMS("Enabling package C8+\n");
> >
> > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
> > + if (HAS_PCH_LPT_LP(dev)) {
> > val = I915_READ(SOUTH_DSPCLK_GATE_D);
> > val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
> > I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
> > @@ -9455,7 +9452,7 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
> > hsw_restore_lcpll(dev_priv);
> > lpt_init_pch_refclk(dev);
> >
> > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
> > + if (HAS_PCH_LPT_LP(dev)) {
> > val = I915_READ(SOUTH_DSPCLK_GATE_D);
> > val |= PCH_LP_PARTITION_LEVEL_DISABLE;
> > I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index fff0c226..ea49661 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6588,7 +6588,7 @@ static void lpt_init_clock_gating(struct drm_device *dev)
> > * TODO: this bit should only be enabled when really needed, then
> > * disabled when not needed anymore in order to save power.
> > */
> > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE)
> > + if (HAS_PCH_LPT_LP(dev))
> > I915_WRITE(SOUTH_DSPCLK_GATE_D,
> > I915_READ(SOUTH_DSPCLK_GATE_D) |
> > PCH_LP_PARTITION_LEVEL_DISABLE);
> > @@ -6603,7 +6603,7 @@ static void lpt_suspend_hw(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > - if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
> > + if (HAS_PCH_LPT_LP(dev)) {
> > uint32_t val = I915_READ(SOUTH_DSPCLK_GATE_D);
> >
> > val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
> > --
> > 2.4.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (3 preceding siblings ...)
2015-08-12 15:44 ` [PATCH 04/11] drm/i915: Add HAS_PCH_LPT_LP() macro ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-26 19:13 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 06/11] drm/i915: Introduce spt_irq_handler() ville.syrjala
` (7 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The PORTA HPD defines are not BXT specific. They also exist on SPT,
and partially already on LPT:LP.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 2 +-
drivers/gpu/drm/i915/i915_reg.h | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8a1e35e..d12106c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1248,7 +1248,7 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
{
switch (port) {
case PORT_A:
- return val & BXT_PORTA_HOTPLUG_LONG_DETECT;
+ return val & PORTA_HOTPLUG_LONG_DETECT;
case PORT_B:
return val & PORTB_HOTPLUG_LONG_DETECT;
case PORT_C:
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ed2d150..0e9990b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6002,11 +6002,11 @@ enum skl_disp_power_wells {
/* digital port hotplug */
#define PCH_PORT_HOTPLUG 0xc4030 /* SHOTPLUG_CTL */
-#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
-#define BXT_PORTA_HOTPLUG_STATUS_MASK (3 << 24)
-#define BXT_PORTA_HOTPLUG_NO_DETECT (0 << 24)
-#define BXT_PORTA_HOTPLUG_SHORT_DETECT (1 << 24)
-#define BXT_PORTA_HOTPLUG_LONG_DETECT (2 << 24)
+#define PORTA_HOTPLUG_ENABLE (1 << 28) /* LPT:LP+ & BXT */
+#define PORTA_HOTPLUG_STATUS_MASK (3 << 24) /* SPT+ & BXT */
+#define PORTA_HOTPLUG_NO_DETECT (0 << 24) /* SPT+ & BXT */
+#define PORTA_HOTPLUG_SHORT_DETECT (1 << 24) /* SPT+ & BXT */
+#define PORTA_HOTPLUG_LONG_DETECT (2 << 24) /* SPT+ & BXT */
#define PORTD_HOTPLUG_ENABLE (1 << 20)
#define PORTD_PULSE_DURATION_2ms (0 << 18)
#define PORTD_PULSE_DURATION_4_5ms (1 << 18)
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines
2015-08-12 15:44 ` [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines ville.syrjala
@ 2015-08-26 19:13 ` Paulo Zanoni
2015-08-26 19:43 ` Ville Syrjälä
2015-08-27 16:39 ` Ville Syrjälä
0 siblings, 2 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-26 19:13 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The PORTA HPD defines are not BXT specific. They also exist on SPT,
> and partially already on LPT:LP.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 8a1e35e..d12106c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1248,7 +1248,7 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
> {
> switch (port) {
> case PORT_A:
> - return val & BXT_PORTA_HOTPLUG_LONG_DETECT;
> + return val & PORTA_HOTPLUG_LONG_DETECT;
> case PORT_B:
> return val & PORTB_HOTPLUG_LONG_DETECT;
> case PORT_C:
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ed2d150..0e9990b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6002,11 +6002,11 @@ enum skl_disp_power_wells {
>
> /* digital port hotplug */
> #define PCH_PORT_HOTPLUG 0xc4030 /* SHOTPLUG_CTL */
> -#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
> -#define BXT_PORTA_HOTPLUG_STATUS_MASK (3 << 24)
> -#define BXT_PORTA_HOTPLUG_NO_DETECT (0 << 24)
> -#define BXT_PORTA_HOTPLUG_SHORT_DETECT (1 << 24)
> -#define BXT_PORTA_HOTPLUG_LONG_DETECT (2 << 24)
> +#define PORTA_HOTPLUG_ENABLE (1 << 28) /* LPT:LP+ & BXT */
Although the doc for LPT _suggests_ this is only for LPT:LP, it
doesn't mark this bit as LPT:LP-specific just like it marks all the
other LPT:LP-specific bits in every register, so I wonder if this is
really LPT:LP or if there's another way to find this out, like some
strap or VBT bit.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> +#define PORTA_HOTPLUG_STATUS_MASK (3 << 24) /* SPT+ & BXT */
> +#define PORTA_HOTPLUG_NO_DETECT (0 << 24) /* SPT+ & BXT */
> +#define PORTA_HOTPLUG_SHORT_DETECT (1 << 24) /* SPT+ & BXT */
> +#define PORTA_HOTPLUG_LONG_DETECT (2 << 24) /* SPT+ & BXT */
> #define PORTD_HOTPLUG_ENABLE (1 << 20)
> #define PORTD_PULSE_DURATION_2ms (0 << 18)
> #define PORTD_PULSE_DURATION_4_5ms (1 << 18)
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines
2015-08-26 19:13 ` Paulo Zanoni
@ 2015-08-26 19:43 ` Ville Syrjälä
2015-08-26 21:59 ` Runyan, Arthur J
2015-08-27 16:39 ` Ville Syrjälä
1 sibling, 1 reply; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-26 19:43 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Runyan, Arthur J
On Wed, Aug 26, 2015 at 04:13:52PM -0300, Paulo Zanoni wrote:
> 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The PORTA HPD defines are not BXT specific. They also exist on SPT,
> > and partially already on LPT:LP.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 2 +-
> > drivers/gpu/drm/i915/i915_reg.h | 10 +++++-----
> > 2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 8a1e35e..d12106c 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1248,7 +1248,7 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
> > {
> > switch (port) {
> > case PORT_A:
> > - return val & BXT_PORTA_HOTPLUG_LONG_DETECT;
> > + return val & PORTA_HOTPLUG_LONG_DETECT;
> > case PORT_B:
> > return val & PORTB_HOTPLUG_LONG_DETECT;
> > case PORT_C:
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index ed2d150..0e9990b 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6002,11 +6002,11 @@ enum skl_disp_power_wells {
> >
> > /* digital port hotplug */
> > #define PCH_PORT_HOTPLUG 0xc4030 /* SHOTPLUG_CTL */
> > -#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
> > -#define BXT_PORTA_HOTPLUG_STATUS_MASK (3 << 24)
> > -#define BXT_PORTA_HOTPLUG_NO_DETECT (0 << 24)
> > -#define BXT_PORTA_HOTPLUG_SHORT_DETECT (1 << 24)
> > -#define BXT_PORTA_HOTPLUG_LONG_DETECT (2 << 24)
> > +#define PORTA_HOTPLUG_ENABLE (1 << 28) /* LPT:LP+ & BXT */
>
> Although the doc for LPT _suggests_ this is only for LPT:LP, it
> doesn't mark this bit as LPT:LP-specific just like it marks all the
> other LPT:LP-specific bits in every register, so I wonder if this is
> really LPT:LP or if there's another way to find this out, like some
> strap or VBT bit.
Hmm. Indeed. There is that note about the enable being in the north
on DevLPT:H. I guess that's what gave me the idea for this patch. But
the rest of the text just talks about PCH being on the same package or
not. Not sure if the two conditions are entirely the same thing.
For HSW the north register says we need this on DevHSW:ULT. For BDW it
refers us to a package type indication in FUSE_STRAP3, but the only
relevant looking bit is the ULT mode bit, which it also says is
currently unused and should be ignored :(
Art, could you help us out here? How should we actually determine
(on HSW/BDW) whether to enable the DDI A HPD in the north, south,
or both?
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> > +#define PORTA_HOTPLUG_STATUS_MASK (3 << 24) /* SPT+ & BXT */
> > +#define PORTA_HOTPLUG_NO_DETECT (0 << 24) /* SPT+ & BXT */
> > +#define PORTA_HOTPLUG_SHORT_DETECT (1 << 24) /* SPT+ & BXT */
> > +#define PORTA_HOTPLUG_LONG_DETECT (2 << 24) /* SPT+ & BXT */
> > #define PORTD_HOTPLUG_ENABLE (1 << 20)
> > #define PORTD_PULSE_DURATION_2ms (0 << 18)
> > #define PORTD_PULSE_DURATION_4_5ms (1 << 18)
> > --
> > 2.4.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines
2015-08-26 19:43 ` Ville Syrjälä
@ 2015-08-26 21:59 ` Runyan, Arthur J
2015-08-27 15:50 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Runyan, Arthur J @ 2015-08-26 21:59 UTC (permalink / raw)
To: Ville Syrjälä, Paulo Zanoni; +Cc: Intel Graphics Development
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>> On Wed, Aug 26, 2015 at 04:13:52PM -0300, Paulo Zanoni wrote:
...
>> Although the doc for LPT _suggests_ this is only for LPT:LP, it
>> doesn't mark this bit as LPT:LP-specific just like it marks all the
>> other LPT:LP-specific bits in every register, so I wonder if this is
>> really LPT:LP or if there's another way to find this out, like some
>> strap or VBT bit.
>
>Hmm. Indeed. There is that note about the enable being in the north
>on DevLPT:H. I guess that's what gave me the idea for this patch. But
>the rest of the text just talks about PCH being on the same package or
>not. Not sure if the two conditions are entirely the same thing.
>
>For HSW the north register says we need this on DevHSW:ULT. For BDW it
>refers us to a package type indication in FUSE_STRAP3, but the only
>relevant looking bit is the ULT mode bit, which it also says is
>currently unused and should be ignored :(
>
>Art, could you help us out here? How should we actually determine
>(on HSW/BDW) whether to enable the DDI A HPD in the north, south,
>or both?
The north and south hotplug control registers have text to try to explain this in the DDI A HPD enable fields, which I pasted below. You always enable north. You additionally enable south if CPU and PCH are in the same package.
"This only applies to systems that have the CPU and PCH in the same package, where the DDI A HPD input is connected to the PCH and the HPD must be enabled in both the North Display Engine Registers HOTPLUG_CTL and the South Display Engine Registers SHOTPLUG_CTL. The HPD status is found in North Display Engine Registers HOTPLUG_CTL.
On systems that have the CPU and PCH in separate packages, the DDI A HPD input is connected to the CPU, and the DDI A HPD input must be enabled in only the North Display Engine Registers HOTPLUG_CTL."
That fuse may not be correct on all SKUs, but I assume you have other ways to recognize what kind of package it is. I originally listed out ULT and ULX, but it became more complicated with BDW.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines
2015-08-26 21:59 ` Runyan, Arthur J
@ 2015-08-27 15:50 ` Ville Syrjälä
2015-08-27 19:34 ` Runyan, Arthur J
0 siblings, 1 reply; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-27 15:50 UTC (permalink / raw)
To: Runyan, Arthur J; +Cc: Intel Graphics Development
On Wed, Aug 26, 2015 at 09:59:13PM +0000, Runyan, Arthur J wrote:
>
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >> On Wed, Aug 26, 2015 at 04:13:52PM -0300, Paulo Zanoni wrote:
> ...
> >> Although the doc for LPT _suggests_ this is only for LPT:LP, it
> >> doesn't mark this bit as LPT:LP-specific just like it marks all the
> >> other LPT:LP-specific bits in every register, so I wonder if this is
> >> really LPT:LP or if there's another way to find this out, like some
> >> strap or VBT bit.
> >
> >Hmm. Indeed. There is that note about the enable being in the north
> >on DevLPT:H. I guess that's what gave me the idea for this patch. But
> >the rest of the text just talks about PCH being on the same package or
> >not. Not sure if the two conditions are entirely the same thing.
> >
> >For HSW the north register says we need this on DevHSW:ULT. For BDW it
> >refers us to a package type indication in FUSE_STRAP3, but the only
> >relevant looking bit is the ULT mode bit, which it also says is
> >currently unused and should be ignored :(
> >
> >Art, could you help us out here? How should we actually determine
> >(on HSW/BDW) whether to enable the DDI A HPD in the north, south,
> >or both?
>
> The north and south hotplug control registers have text to try to explain this in the DDI A HPD enable fields, which I pasted below. You always enable north. You additionally enable south if CPU and PCH are in the same package.
OK thanks. That more or less matches my original understanding.
>
> "This only applies to systems that have the CPU and PCH in the same package, where the DDI A HPD input is connected to the PCH and the HPD must be enabled in both the North Display Engine Registers HOTPLUG_CTL and the South Display Engine Registers SHOTPLUG_CTL. The HPD status is found in North Display Engine Registers HOTPLUG_CTL.
>
> On systems that have the CPU and PCH in separate packages, the DDI A HPD input is connected to the CPU, and the DDI A HPD input must be enabled in only the North Display Engine Registers HOTPLUG_CTL."
>
> That fuse may not be correct on all SKUs, but I assume you have other ways to recognize what kind of package it is. I originally listed out ULT and ULX, but it became more complicated with BDW.
I'm not aware of any way of identifying the package type. My original
assumption was LPT-LP -> MCP, LPT-H -> separate package, and most of
the material I've managed to dig up would seem to support that. But
it's hard to be sure.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines
2015-08-27 15:50 ` Ville Syrjälä
@ 2015-08-27 19:34 ` Runyan, Arthur J
2015-08-27 19:52 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Runyan, Arthur J @ 2015-08-27 19:34 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>> That fuse may not be correct on all SKUs, but I assume you have other ways to
>recognize what kind of package it is. I originally listed out ULT and ULX, but it
>became more complicated with BDW.
>
>I'm not aware of any way of identifying the package type. My original
>assumption was LPT-LP -> MCP, LPT-H -> separate package, and most of
>the material I've managed to dig up would seem to support that. But
>it's hard to be sure.
That is correct.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines
2015-08-27 19:34 ` Runyan, Arthur J
@ 2015-08-27 19:52 ` Ville Syrjälä
0 siblings, 0 replies; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-27 19:52 UTC (permalink / raw)
To: Runyan, Arthur J; +Cc: Intel Graphics Development
On Thu, Aug 27, 2015 at 07:34:41PM +0000, Runyan, Arthur J wrote:
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >> That fuse may not be correct on all SKUs, but I assume you have other ways to
> >recognize what kind of package it is. I originally listed out ULT and ULX, but it
> >became more complicated with BDW.
> >
> >I'm not aware of any way of identifying the package type. My original
> >assumption was LPT-LP -> MCP, LPT-H -> separate package, and most of
> >the material I've managed to dig up would seem to support that. But
> >it's hard to be sure.
>
> That is correct.
Excellent. Thanks Art.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines
2015-08-26 19:13 ` Paulo Zanoni
2015-08-26 19:43 ` Ville Syrjälä
@ 2015-08-27 16:39 ` Ville Syrjälä
1 sibling, 0 replies; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-27 16:39 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development
On Wed, Aug 26, 2015 at 04:13:52PM -0300, Paulo Zanoni wrote:
> 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The PORTA HPD defines are not BXT specific. They also exist on SPT,
> > and partially already on LPT:LP.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 2 +-
> > drivers/gpu/drm/i915/i915_reg.h | 10 +++++-----
> > 2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 8a1e35e..d12106c 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1248,7 +1248,7 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
> > {
> > switch (port) {
> > case PORT_A:
> > - return val & BXT_PORTA_HOTPLUG_LONG_DETECT;
> > + return val & PORTA_HOTPLUG_LONG_DETECT;
> > case PORT_B:
> > return val & PORTB_HOTPLUG_LONG_DETECT;
> > case PORT_C:
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index ed2d150..0e9990b 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6002,11 +6002,11 @@ enum skl_disp_power_wells {
> >
> > /* digital port hotplug */
> > #define PCH_PORT_HOTPLUG 0xc4030 /* SHOTPLUG_CTL */
> > -#define BXT_PORTA_HOTPLUG_ENABLE (1 << 28)
> > -#define BXT_PORTA_HOTPLUG_STATUS_MASK (3 << 24)
> > -#define BXT_PORTA_HOTPLUG_NO_DETECT (0 << 24)
> > -#define BXT_PORTA_HOTPLUG_SHORT_DETECT (1 << 24)
> > -#define BXT_PORTA_HOTPLUG_LONG_DETECT (2 << 24)
> > +#define PORTA_HOTPLUG_ENABLE (1 << 28) /* LPT:LP+ & BXT */
>
> Although the doc for LPT _suggests_ this is only for LPT:LP, it
> doesn't mark this bit as LPT:LP-specific just like it marks all the
> other LPT:LP-specific bits in every register, so I wonder if this is
> really LPT:LP or if there's another way to find this out, like some
> strap or VBT bit.
Just did a quick experiment and the bit won't stick on my desktop machine
with LPT-H, but it will on a ULT with LPT-LP. So looks like that part of
the patch is at least correct, unless some strap would affect whether
the bit even sticks or not.
>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> > +#define PORTA_HOTPLUG_STATUS_MASK (3 << 24) /* SPT+ & BXT */
> > +#define PORTA_HOTPLUG_NO_DETECT (0 << 24) /* SPT+ & BXT */
> > +#define PORTA_HOTPLUG_SHORT_DETECT (1 << 24) /* SPT+ & BXT */
> > +#define PORTA_HOTPLUG_LONG_DETECT (2 << 24) /* SPT+ & BXT */
> > #define PORTD_HOTPLUG_ENABLE (1 << 20)
> > #define PORTD_PULSE_DURATION_2ms (0 << 18)
> > #define PORTD_PULSE_DURATION_4_5ms (1 << 18)
> > --
> > 2.4.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 06/11] drm/i915: Introduce spt_irq_handler()
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (4 preceding siblings ...)
2015-08-12 15:44 ` [PATCH 05/11] drm/i915: Rename BXT PORTA HPD defines ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-26 21:44 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 07/11] drm/i915: Add port A HPD support for ILK/SNB ville.syrjala
` (6 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Starting from SPT the only interrupts living in the south are GMBUS and
HPD. What's worse some of the SPT specific new bits conflict with some
other bits on earlier PCH generations. So better not use the
cpt_irq_handler() for SPT+ anymore.
Also kill the hand rolled port E handling with something more
standardish. This also avoids accidentally confusing port B and port E
long pulses since the bits occupy the same positions, just in different
registers.
Also add a comment noting that the short pulse duration bits are
reserved on LPT+. The 2ms value we program is 0, so no issue wrt. the
MBZ in the spec.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++++++++++-------------
1 file changed, 83 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d12106c..e2485bd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1260,6 +1260,16 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
}
}
+static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
+{
+ switch (port) {
+ case PORT_E:
+ return val & PORTE_HOTPLUG_LONG_DETECT;
+ default:
+ return false;
+ }
+}
+
static bool pch_port_hotplug_long_detect(enum port port, u32 val)
{
switch (port) {
@@ -1269,8 +1279,6 @@ static bool pch_port_hotplug_long_detect(enum port port, u32 val)
return val & PORTC_HOTPLUG_LONG_DETECT;
case PORT_D:
return val & PORTD_HOTPLUG_LONG_DETECT;
- case PORT_E:
- return val & PORTE_HOTPLUG_LONG_DETECT;
default:
return false;
}
@@ -1771,12 +1779,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
{
struct drm_i915_private *dev_priv = dev->dev_private;
int pipe;
- u32 hotplug_trigger;
-
- if (HAS_PCH_SPT(dev))
- hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT;
- else
- hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
+ u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
if (hotplug_trigger) {
u32 dig_hotplug_reg, pin_mask, long_mask;
@@ -1784,22 +1787,10 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
- if (HAS_PCH_SPT(dev)) {
- intel_get_hpd_pins(&pin_mask, &long_mask,
- hotplug_trigger,
- dig_hotplug_reg, hpd_spt,
- pch_port_hotplug_long_detect);
-
- /* detect PORTE HP event */
- dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
- if (pch_port_hotplug_long_detect(PORT_E,
- dig_hotplug_reg))
- long_mask |= 1 << HPD_PORT_E;
- } else
- intel_get_hpd_pins(&pin_mask, &long_mask,
- hotplug_trigger,
- dig_hotplug_reg, hpd_cpt,
- pch_port_hotplug_long_detect);
+ intel_get_hpd_pins(&pin_mask, &long_mask,
+ hotplug_trigger,
+ dig_hotplug_reg, hpd_cpt,
+ pch_port_hotplug_long_detect);
intel_hpd_irq_handler(dev, pin_mask, long_mask);
}
@@ -1833,6 +1824,42 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
cpt_serr_int_handler(dev);
}
+static void spt_irq_handler(struct drm_device *dev, u32 pch_iir)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
+ ~SDE_PORTE_HOTPLUG_SPT;
+ u32 hotplug2_trigger = pch_iir & SDE_PORTE_HOTPLUG_SPT;
+
+ if (hotplug_trigger) {
+ u32 dig_hotplug_reg, pin_mask, long_mask;
+
+ dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
+ I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
+
+ intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+ dig_hotplug_reg, hpd_spt,
+ pch_port_hotplug_long_detect);
+ intel_hpd_irq_handler(dev, pin_mask, long_mask);
+ }
+
+ if (hotplug2_trigger) {
+ u32 dig_hotplug_reg, pin_mask, long_mask;
+
+ dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
+ I915_WRITE(PCH_PORT_HOTPLUG2, dig_hotplug_reg);
+
+ intel_get_hpd_pins(&pin_mask, &long_mask, hotplug2_trigger,
+ dig_hotplug_reg, hpd_spt,
+ spt_port_hotplug2_long_detect);
+
+ intel_hpd_irq_handler(dev, pin_mask, long_mask);
+ }
+
+ if (pch_iir & SDE_GMBUS_CPT)
+ gmbus_irq_handler(dev);
+}
+
static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2151,7 +2178,11 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
if (pch_iir) {
I915_WRITE(SDEIIR, pch_iir);
ret = IRQ_HANDLED;
- cpt_irq_handler(dev, pch_iir);
+
+ if (HAS_PCH_SPT(dev_priv))
+ spt_irq_handler(dev, pch_iir);
+ else
+ cpt_irq_handler(dev, pch_iir);
} else
DRM_ERROR("The master control interrupt lied (SDE)!\n");
@@ -3033,9 +3064,6 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
if (HAS_PCH_IBX(dev)) {
hotplug_irqs = SDE_HOTPLUG_MASK;
enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ibx);
- } else if (HAS_PCH_SPT(dev)) {
- hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
- enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
} else {
hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_cpt);
@@ -3045,9 +3073,8 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
/*
* Enable digital hotplug on the PCH, and configure the DP short pulse
- * duration to 2ms (which is the minimum in the Display Port spec)
- *
- * This register is the same on all known PCH chips.
+ * duration to 2ms (which is the minimum in the Display Port spec).
+ * The pulse duration bits are reserved on LPT+.
*/
hotplug = I915_READ(PCH_PORT_HOTPLUG);
hotplug &= ~(PORTD_PULSE_DURATION_MASK|PORTC_PULSE_DURATION_MASK|PORTB_PULSE_DURATION_MASK);
@@ -3055,13 +3082,27 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
+}
+
+static void spt_hpd_irq_setup(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 hotplug_irqs, hotplug, enabled_irqs;
+
+ hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
+ enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
+
+ ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
+
+ /* Enable digital hotplug on the PCH */
+ hotplug = I915_READ(PCH_PORT_HOTPLUG);
+ hotplug |= PORTD_HOTPLUG_ENABLE | PORTC_HOTPLUG_ENABLE |
+ PORTB_HOTPLUG_ENABLE;
+ I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
- /* enable SPT PORTE hot plug */
- if (HAS_PCH_SPT(dev)) {
- hotplug = I915_READ(PCH_PORT_HOTPLUG2);
- hotplug |= PORTE_HOTPLUG_ENABLE;
- I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
- }
+ hotplug = I915_READ(PCH_PORT_HOTPLUG2);
+ hotplug |= PORTE_HOTPLUG_ENABLE;
+ I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
}
static void bxt_hpd_irq_setup(struct drm_device *dev)
@@ -4166,10 +4207,12 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
dev->driver->irq_uninstall = gen8_irq_uninstall;
dev->driver->enable_vblank = gen8_enable_vblank;
dev->driver->disable_vblank = gen8_disable_vblank;
- if (HAS_PCH_SPLIT(dev))
- dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
- else
+ if (IS_BROXTON(dev))
dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
+ else if (HAS_PCH_SPT(dev))
+ dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
+ else
+ dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
} else if (HAS_PCH_SPLIT(dev)) {
dev->driver->irq_handler = ironlake_irq_handler;
dev->driver->irq_preinstall = ironlake_irq_reset;
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 06/11] drm/i915: Introduce spt_irq_handler()
2015-08-12 15:44 ` [PATCH 06/11] drm/i915: Introduce spt_irq_handler() ville.syrjala
@ 2015-08-26 21:44 ` Paulo Zanoni
2015-08-27 7:38 ` Jani Nikula
0 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-26 21:44 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Starting from SPT the only interrupts living in the south are GMBUS and
> HPD. What's worse some of the SPT specific new bits conflict with some
> other bits on earlier PCH generations. So better not use the
> cpt_irq_handler() for SPT+ anymore.
>
> Also kill the hand rolled port E handling with something more
> standardish. This also avoids accidentally confusing port B and port E
> long pulses since the bits occupy the same positions, just in different
> registers.
>
> Also add a comment noting that the short pulse duration bits are
> reserved on LPT+. The 2ms value we program is 0, so no issue wrt. the
> MBZ in the spec.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++++++++++-------------
> 1 file changed, 83 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d12106c..e2485bd 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1260,6 +1260,16 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
> }
> }
>
> +static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
> +{
> + switch (port) {
> + case PORT_E:
> + return val & PORTE_HOTPLUG_LONG_DETECT;
> + default:
> + return false;
> + }
> +}
> +
> static bool pch_port_hotplug_long_detect(enum port port, u32 val)
> {
> switch (port) {
> @@ -1269,8 +1279,6 @@ static bool pch_port_hotplug_long_detect(enum port port, u32 val)
> return val & PORTC_HOTPLUG_LONG_DETECT;
> case PORT_D:
> return val & PORTD_HOTPLUG_LONG_DETECT;
> - case PORT_E:
> - return val & PORTE_HOTPLUG_LONG_DETECT;
> default:
> return false;
> }
> @@ -1771,12 +1779,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> int pipe;
> - u32 hotplug_trigger;
> -
> - if (HAS_PCH_SPT(dev))
> - hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT;
> - else
> - hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
> + u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
>
> if (hotplug_trigger) {
> u32 dig_hotplug_reg, pin_mask, long_mask;
> @@ -1784,22 +1787,10 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>
> - if (HAS_PCH_SPT(dev)) {
> - intel_get_hpd_pins(&pin_mask, &long_mask,
> - hotplug_trigger,
> - dig_hotplug_reg, hpd_spt,
> - pch_port_hotplug_long_detect);
> -
> - /* detect PORTE HP event */
> - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
> - if (pch_port_hotplug_long_detect(PORT_E,
> - dig_hotplug_reg))
> - long_mask |= 1 << HPD_PORT_E;
> - } else
> - intel_get_hpd_pins(&pin_mask, &long_mask,
> - hotplug_trigger,
> - dig_hotplug_reg, hpd_cpt,
> - pch_port_hotplug_long_detect);
> + intel_get_hpd_pins(&pin_mask, &long_mask,
> + hotplug_trigger,
> + dig_hotplug_reg, hpd_cpt,
> + pch_port_hotplug_long_detect);
>
> intel_hpd_irq_handler(dev, pin_mask, long_mask);
> }
> @@ -1833,6 +1824,42 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> cpt_serr_int_handler(dev);
> }
>
> +static void spt_irq_handler(struct drm_device *dev, u32 pch_iir)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
> + ~SDE_PORTE_HOTPLUG_SPT;
> + u32 hotplug2_trigger = pch_iir & SDE_PORTE_HOTPLUG_SPT;
> +
> + if (hotplug_trigger) {
> + u32 dig_hotplug_reg, pin_mask, long_mask;
> +
> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> +
> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> + dig_hotplug_reg, hpd_spt,
> + pch_port_hotplug_long_detect);
> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> + }
> +
> + if (hotplug2_trigger) {
> + u32 dig_hotplug_reg, pin_mask, long_mask;
> +
> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
> + I915_WRITE(PCH_PORT_HOTPLUG2, dig_hotplug_reg);
> +
> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug2_trigger,
> + dig_hotplug_reg, hpd_spt,
> + spt_port_hotplug2_long_detect);
> +
> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> + }
Instead of calling intel_hpd_irq_handler() twice, can't you just do
something like:
intel_hpd_irq_hanlder(dev, pin_mask1 | pin_mask2, long_mask1 | long_mask2) ?
Everything else looks good (except for the fact that I would have
split this patch in like 5 different patches).
> +
> + if (pch_iir & SDE_GMBUS_CPT)
> + gmbus_irq_handler(dev);
> +}
> +
> static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2151,7 +2178,11 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
> if (pch_iir) {
> I915_WRITE(SDEIIR, pch_iir);
> ret = IRQ_HANDLED;
> - cpt_irq_handler(dev, pch_iir);
> +
> + if (HAS_PCH_SPT(dev_priv))
> + spt_irq_handler(dev, pch_iir);
> + else
> + cpt_irq_handler(dev, pch_iir);
> } else
> DRM_ERROR("The master control interrupt lied (SDE)!\n");
>
> @@ -3033,9 +3064,6 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
> if (HAS_PCH_IBX(dev)) {
> hotplug_irqs = SDE_HOTPLUG_MASK;
> enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ibx);
> - } else if (HAS_PCH_SPT(dev)) {
> - hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
> - enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
> } else {
> hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
> enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_cpt);
> @@ -3045,9 +3073,8 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
>
> /*
> * Enable digital hotplug on the PCH, and configure the DP short pulse
> - * duration to 2ms (which is the minimum in the Display Port spec)
> - *
> - * This register is the same on all known PCH chips.
> + * duration to 2ms (which is the minimum in the Display Port spec).
> + * The pulse duration bits are reserved on LPT+.
> */
> hotplug = I915_READ(PCH_PORT_HOTPLUG);
> hotplug &= ~(PORTD_PULSE_DURATION_MASK|PORTC_PULSE_DURATION_MASK|PORTB_PULSE_DURATION_MASK);
> @@ -3055,13 +3082,27 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
> hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
> hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
> I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
> +}
> +
> +static void spt_hpd_irq_setup(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 hotplug_irqs, hotplug, enabled_irqs;
> +
> + hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
> +
> + ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
> +
> + /* Enable digital hotplug on the PCH */
> + hotplug = I915_READ(PCH_PORT_HOTPLUG);
> + hotplug |= PORTD_HOTPLUG_ENABLE | PORTC_HOTPLUG_ENABLE |
> + PORTB_HOTPLUG_ENABLE;
> + I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>
> - /* enable SPT PORTE hot plug */
> - if (HAS_PCH_SPT(dev)) {
> - hotplug = I915_READ(PCH_PORT_HOTPLUG2);
> - hotplug |= PORTE_HOTPLUG_ENABLE;
> - I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
> - }
> + hotplug = I915_READ(PCH_PORT_HOTPLUG2);
> + hotplug |= PORTE_HOTPLUG_ENABLE;
> + I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
> }
>
> static void bxt_hpd_irq_setup(struct drm_device *dev)
> @@ -4166,10 +4207,12 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> dev->driver->irq_uninstall = gen8_irq_uninstall;
> dev->driver->enable_vblank = gen8_enable_vblank;
> dev->driver->disable_vblank = gen8_disable_vblank;
> - if (HAS_PCH_SPLIT(dev))
> - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> - else
> + if (IS_BROXTON(dev))
> dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
> + else if (HAS_PCH_SPT(dev))
> + dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
> + else
> + dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> } else if (HAS_PCH_SPLIT(dev)) {
> dev->driver->irq_handler = ironlake_irq_handler;
> dev->driver->irq_preinstall = ironlake_irq_reset;
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 06/11] drm/i915: Introduce spt_irq_handler()
2015-08-26 21:44 ` Paulo Zanoni
@ 2015-08-27 7:38 ` Jani Nikula
2015-08-27 16:13 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Jani Nikula @ 2015-08-27 7:38 UTC (permalink / raw)
To: Paulo Zanoni, Ville Syrjälä; +Cc: Intel Graphics Development
On Thu, 27 Aug 2015, Paulo Zanoni <przanoni@gmail.com> wrote:
> 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Starting from SPT the only interrupts living in the south are GMBUS and
>> HPD. What's worse some of the SPT specific new bits conflict with some
>> other bits on earlier PCH generations. So better not use the
>> cpt_irq_handler() for SPT+ anymore.
>>
>> Also kill the hand rolled port E handling with something more
>> standardish. This also avoids accidentally confusing port B and port E
>> long pulses since the bits occupy the same positions, just in different
>> registers.
>>
>> Also add a comment noting that the short pulse duration bits are
>> reserved on LPT+. The 2ms value we program is 0, so no issue wrt. the
>> MBZ in the spec.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++++++++++-------------
>> 1 file changed, 83 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index d12106c..e2485bd 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1260,6 +1260,16 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
>> }
>> }
>>
>> +static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
>> +{
>> + switch (port) {
>> + case PORT_E:
>> + return val & PORTE_HOTPLUG_LONG_DETECT;
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> static bool pch_port_hotplug_long_detect(enum port port, u32 val)
>> {
>> switch (port) {
>> @@ -1269,8 +1279,6 @@ static bool pch_port_hotplug_long_detect(enum port port, u32 val)
>> return val & PORTC_HOTPLUG_LONG_DETECT;
>> case PORT_D:
>> return val & PORTD_HOTPLUG_LONG_DETECT;
>> - case PORT_E:
>> - return val & PORTE_HOTPLUG_LONG_DETECT;
>> default:
>> return false;
>> }
>> @@ -1771,12 +1779,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> int pipe;
>> - u32 hotplug_trigger;
>> -
>> - if (HAS_PCH_SPT(dev))
>> - hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT;
>> - else
>> - hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
>> + u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
>>
>> if (hotplug_trigger) {
>> u32 dig_hotplug_reg, pin_mask, long_mask;
>> @@ -1784,22 +1787,10 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>> dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
>> I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>>
>> - if (HAS_PCH_SPT(dev)) {
>> - intel_get_hpd_pins(&pin_mask, &long_mask,
>> - hotplug_trigger,
>> - dig_hotplug_reg, hpd_spt,
>> - pch_port_hotplug_long_detect);
>> -
>> - /* detect PORTE HP event */
>> - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
>> - if (pch_port_hotplug_long_detect(PORT_E,
>> - dig_hotplug_reg))
>> - long_mask |= 1 << HPD_PORT_E;
>> - } else
>> - intel_get_hpd_pins(&pin_mask, &long_mask,
>> - hotplug_trigger,
>> - dig_hotplug_reg, hpd_cpt,
>> - pch_port_hotplug_long_detect);
>> + intel_get_hpd_pins(&pin_mask, &long_mask,
>> + hotplug_trigger,
>> + dig_hotplug_reg, hpd_cpt,
>> + pch_port_hotplug_long_detect);
>>
>> intel_hpd_irq_handler(dev, pin_mask, long_mask);
>> }
>> @@ -1833,6 +1824,42 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>> cpt_serr_int_handler(dev);
>> }
>>
>> +static void spt_irq_handler(struct drm_device *dev, u32 pch_iir)
>> +{
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
>> + ~SDE_PORTE_HOTPLUG_SPT;
>> + u32 hotplug2_trigger = pch_iir & SDE_PORTE_HOTPLUG_SPT;
>> +
>> + if (hotplug_trigger) {
>> + u32 dig_hotplug_reg, pin_mask, long_mask;
>> +
>> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
>> + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
>> +
>> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
>> + dig_hotplug_reg, hpd_spt,
>> + pch_port_hotplug_long_detect);
>> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
>> + }
>> +
>> + if (hotplug2_trigger) {
>> + u32 dig_hotplug_reg, pin_mask, long_mask;
>> +
>> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
>> + I915_WRITE(PCH_PORT_HOTPLUG2, dig_hotplug_reg);
>> +
>> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug2_trigger,
>> + dig_hotplug_reg, hpd_spt,
>> + spt_port_hotplug2_long_detect);
>> +
>> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
>> + }
>
> Instead of calling intel_hpd_irq_handler() twice, can't you just do
> something like:
> intel_hpd_irq_hanlder(dev, pin_mask1 | pin_mask2, long_mask1 | long_mask2) ?
And if you reverted
commit fd63e2a972c670887e5e8a08440111d3812c0996
Author: Imre Deak <imre.deak@intel.com>
Date: Tue Jul 21 15:32:44 2015 -0700
drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
and added another version for spt, this would all be pretty and each of
the functions would do exactly what is required and you could have
meaningful, platform specific debug logging in each of the functions.
By focusing on deduplicating a few lines in get_hpd_pins, the callers
(which are platform specific code to begin with) get more complicated.
BR,
Jani.
>
> Everything else looks good (except for the fact that I would have
> split this patch in like 5 different patches).
>
>> +
>> + if (pch_iir & SDE_GMBUS_CPT)
>> + gmbus_irq_handler(dev);
>> +}
>> +
>> static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -2151,7 +2178,11 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>> if (pch_iir) {
>> I915_WRITE(SDEIIR, pch_iir);
>> ret = IRQ_HANDLED;
>> - cpt_irq_handler(dev, pch_iir);
>> +
>> + if (HAS_PCH_SPT(dev_priv))
>> + spt_irq_handler(dev, pch_iir);
>> + else
>> + cpt_irq_handler(dev, pch_iir);
>> } else
>> DRM_ERROR("The master control interrupt lied (SDE)!\n");
>>
>> @@ -3033,9 +3064,6 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
>> if (HAS_PCH_IBX(dev)) {
>> hotplug_irqs = SDE_HOTPLUG_MASK;
>> enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ibx);
>> - } else if (HAS_PCH_SPT(dev)) {
>> - hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
>> - enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
>> } else {
>> hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
>> enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_cpt);
>> @@ -3045,9 +3073,8 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
>>
>> /*
>> * Enable digital hotplug on the PCH, and configure the DP short pulse
>> - * duration to 2ms (which is the minimum in the Display Port spec)
>> - *
>> - * This register is the same on all known PCH chips.
>> + * duration to 2ms (which is the minimum in the Display Port spec).
>> + * The pulse duration bits are reserved on LPT+.
>> */
>> hotplug = I915_READ(PCH_PORT_HOTPLUG);
>> hotplug &= ~(PORTD_PULSE_DURATION_MASK|PORTC_PULSE_DURATION_MASK|PORTB_PULSE_DURATION_MASK);
>> @@ -3055,13 +3082,27 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
>> hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
>> hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
>> I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>> +}
>> +
>> +static void spt_hpd_irq_setup(struct drm_device *dev)
>> +{
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> + u32 hotplug_irqs, hotplug, enabled_irqs;
>> +
>> + hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
>> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
>> +
>> + ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
>> +
>> + /* Enable digital hotplug on the PCH */
>> + hotplug = I915_READ(PCH_PORT_HOTPLUG);
>> + hotplug |= PORTD_HOTPLUG_ENABLE | PORTC_HOTPLUG_ENABLE |
>> + PORTB_HOTPLUG_ENABLE;
>> + I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>>
>> - /* enable SPT PORTE hot plug */
>> - if (HAS_PCH_SPT(dev)) {
>> - hotplug = I915_READ(PCH_PORT_HOTPLUG2);
>> - hotplug |= PORTE_HOTPLUG_ENABLE;
>> - I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
>> - }
>> + hotplug = I915_READ(PCH_PORT_HOTPLUG2);
>> + hotplug |= PORTE_HOTPLUG_ENABLE;
>> + I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
>> }
>>
>> static void bxt_hpd_irq_setup(struct drm_device *dev)
>> @@ -4166,10 +4207,12 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>> dev->driver->irq_uninstall = gen8_irq_uninstall;
>> dev->driver->enable_vblank = gen8_enable_vblank;
>> dev->driver->disable_vblank = gen8_disable_vblank;
>> - if (HAS_PCH_SPLIT(dev))
>> - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
>> - else
>> + if (IS_BROXTON(dev))
>> dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
>> + else if (HAS_PCH_SPT(dev))
>> + dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
>> + else
>> + dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
>> } else if (HAS_PCH_SPLIT(dev)) {
>> dev->driver->irq_handler = ironlake_irq_handler;
>> dev->driver->irq_preinstall = ironlake_irq_reset;
>> --
>> 2.4.6
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 06/11] drm/i915: Introduce spt_irq_handler()
2015-08-27 7:38 ` Jani Nikula
@ 2015-08-27 16:13 ` Ville Syrjälä
2015-08-27 17:52 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-27 16:13 UTC (permalink / raw)
To: Jani Nikula; +Cc: Intel Graphics Development
On Thu, Aug 27, 2015 at 10:38:25AM +0300, Jani Nikula wrote:
> On Thu, 27 Aug 2015, Paulo Zanoni <przanoni@gmail.com> wrote:
> > 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Starting from SPT the only interrupts living in the south are GMBUS and
> >> HPD. What's worse some of the SPT specific new bits conflict with some
> >> other bits on earlier PCH generations. So better not use the
> >> cpt_irq_handler() for SPT+ anymore.
> >>
> >> Also kill the hand rolled port E handling with something more
> >> standardish. This also avoids accidentally confusing port B and port E
> >> long pulses since the bits occupy the same positions, just in different
> >> registers.
> >>
> >> Also add a comment noting that the short pulse duration bits are
> >> reserved on LPT+. The 2ms value we program is 0, so no issue wrt. the
> >> MBZ in the spec.
> >>
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++++++++++-------------
> >> 1 file changed, 83 insertions(+), 40 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> index d12106c..e2485bd 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -1260,6 +1260,16 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
> >> }
> >> }
> >>
> >> +static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
> >> +{
> >> + switch (port) {
> >> + case PORT_E:
> >> + return val & PORTE_HOTPLUG_LONG_DETECT;
> >> + default:
> >> + return false;
> >> + }
> >> +}
> >> +
> >> static bool pch_port_hotplug_long_detect(enum port port, u32 val)
> >> {
> >> switch (port) {
> >> @@ -1269,8 +1279,6 @@ static bool pch_port_hotplug_long_detect(enum port port, u32 val)
> >> return val & PORTC_HOTPLUG_LONG_DETECT;
> >> case PORT_D:
> >> return val & PORTD_HOTPLUG_LONG_DETECT;
> >> - case PORT_E:
> >> - return val & PORTE_HOTPLUG_LONG_DETECT;
> >> default:
> >> return false;
> >> }
> >> @@ -1771,12 +1779,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> >> {
> >> struct drm_i915_private *dev_priv = dev->dev_private;
> >> int pipe;
> >> - u32 hotplug_trigger;
> >> -
> >> - if (HAS_PCH_SPT(dev))
> >> - hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT;
> >> - else
> >> - hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
> >> + u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
> >>
> >> if (hotplug_trigger) {
> >> u32 dig_hotplug_reg, pin_mask, long_mask;
> >> @@ -1784,22 +1787,10 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> >> dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> >> I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> >>
> >> - if (HAS_PCH_SPT(dev)) {
> >> - intel_get_hpd_pins(&pin_mask, &long_mask,
> >> - hotplug_trigger,
> >> - dig_hotplug_reg, hpd_spt,
> >> - pch_port_hotplug_long_detect);
> >> -
> >> - /* detect PORTE HP event */
> >> - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
> >> - if (pch_port_hotplug_long_detect(PORT_E,
> >> - dig_hotplug_reg))
> >> - long_mask |= 1 << HPD_PORT_E;
> >> - } else
> >> - intel_get_hpd_pins(&pin_mask, &long_mask,
> >> - hotplug_trigger,
> >> - dig_hotplug_reg, hpd_cpt,
> >> - pch_port_hotplug_long_detect);
> >> + intel_get_hpd_pins(&pin_mask, &long_mask,
> >> + hotplug_trigger,
> >> + dig_hotplug_reg, hpd_cpt,
> >> + pch_port_hotplug_long_detect);
> >>
> >> intel_hpd_irq_handler(dev, pin_mask, long_mask);
> >> }
> >> @@ -1833,6 +1824,42 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> >> cpt_serr_int_handler(dev);
> >> }
> >>
> >> +static void spt_irq_handler(struct drm_device *dev, u32 pch_iir)
> >> +{
> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> + u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
> >> + ~SDE_PORTE_HOTPLUG_SPT;
> >> + u32 hotplug2_trigger = pch_iir & SDE_PORTE_HOTPLUG_SPT;
> >> +
> >> + if (hotplug_trigger) {
> >> + u32 dig_hotplug_reg, pin_mask, long_mask;
> >> +
> >> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> >> + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> >> +
> >> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> >> + dig_hotplug_reg, hpd_spt,
> >> + pch_port_hotplug_long_detect);
> >> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> >> + }
> >> +
> >> + if (hotplug2_trigger) {
> >> + u32 dig_hotplug_reg, pin_mask, long_mask;
> >> +
> >> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
> >> + I915_WRITE(PCH_PORT_HOTPLUG2, dig_hotplug_reg);
> >> +
> >> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug2_trigger,
> >> + dig_hotplug_reg, hpd_spt,
> >> + spt_port_hotplug2_long_detect);
> >> +
> >> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> >> + }
> >
> > Instead of calling intel_hpd_irq_handler() twice, can't you just do
> > something like:
> > intel_hpd_irq_hanlder(dev, pin_mask1 | pin_mask2, long_mask1 | long_mask2) ?
>
>
> And if you reverted
>
> commit fd63e2a972c670887e5e8a08440111d3812c0996
> Author: Imre Deak <imre.deak@intel.com>
> Date: Tue Jul 21 15:32:44 2015 -0700
>
> drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
>
> and added another version for spt, this would all be pretty and each of
> the functions would do exactly what is required and you could have
> meaningful, platform specific debug logging in each of the functions.
>
> By focusing on deduplicating a few lines in get_hpd_pins, the callers
> (which are platform specific code to begin with) get more complicated.
OK, let me have a look at this mess in a bit more detail and see what can
be done. Paulo's idea at least is simple enough to do, so I'll do at
least that.
>
> BR,
> Jani.
>
>
>
> >
> > Everything else looks good (except for the fact that I would have
> > split this patch in like 5 different patches).
> >
> >> +
> >> + if (pch_iir & SDE_GMBUS_CPT)
> >> + gmbus_irq_handler(dev);
> >> +}
> >> +
> >> static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
> >> {
> >> struct drm_i915_private *dev_priv = dev->dev_private;
> >> @@ -2151,7 +2178,11 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
> >> if (pch_iir) {
> >> I915_WRITE(SDEIIR, pch_iir);
> >> ret = IRQ_HANDLED;
> >> - cpt_irq_handler(dev, pch_iir);
> >> +
> >> + if (HAS_PCH_SPT(dev_priv))
> >> + spt_irq_handler(dev, pch_iir);
> >> + else
> >> + cpt_irq_handler(dev, pch_iir);
> >> } else
> >> DRM_ERROR("The master control interrupt lied (SDE)!\n");
> >>
> >> @@ -3033,9 +3064,6 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
> >> if (HAS_PCH_IBX(dev)) {
> >> hotplug_irqs = SDE_HOTPLUG_MASK;
> >> enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ibx);
> >> - } else if (HAS_PCH_SPT(dev)) {
> >> - hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
> >> - enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
> >> } else {
> >> hotplug_irqs = SDE_HOTPLUG_MASK_CPT;
> >> enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_cpt);
> >> @@ -3045,9 +3073,8 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
> >>
> >> /*
> >> * Enable digital hotplug on the PCH, and configure the DP short pulse
> >> - * duration to 2ms (which is the minimum in the Display Port spec)
> >> - *
> >> - * This register is the same on all known PCH chips.
> >> + * duration to 2ms (which is the minimum in the Display Port spec).
> >> + * The pulse duration bits are reserved on LPT+.
> >> */
> >> hotplug = I915_READ(PCH_PORT_HOTPLUG);
> >> hotplug &= ~(PORTD_PULSE_DURATION_MASK|PORTC_PULSE_DURATION_MASK|PORTB_PULSE_DURATION_MASK);
> >> @@ -3055,13 +3082,27 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
> >> hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
> >> hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
> >> I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
> >> +}
> >> +
> >> +static void spt_hpd_irq_setup(struct drm_device *dev)
> >> +{
> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> + u32 hotplug_irqs, hotplug, enabled_irqs;
> >> +
> >> + hotplug_irqs = SDE_HOTPLUG_MASK_SPT;
> >> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_spt);
> >> +
> >> + ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
> >> +
> >> + /* Enable digital hotplug on the PCH */
> >> + hotplug = I915_READ(PCH_PORT_HOTPLUG);
> >> + hotplug |= PORTD_HOTPLUG_ENABLE | PORTC_HOTPLUG_ENABLE |
> >> + PORTB_HOTPLUG_ENABLE;
> >> + I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
> >>
> >> - /* enable SPT PORTE hot plug */
> >> - if (HAS_PCH_SPT(dev)) {
> >> - hotplug = I915_READ(PCH_PORT_HOTPLUG2);
> >> - hotplug |= PORTE_HOTPLUG_ENABLE;
> >> - I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
> >> - }
> >> + hotplug = I915_READ(PCH_PORT_HOTPLUG2);
> >> + hotplug |= PORTE_HOTPLUG_ENABLE;
> >> + I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
> >> }
> >>
> >> static void bxt_hpd_irq_setup(struct drm_device *dev)
> >> @@ -4166,10 +4207,12 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> >> dev->driver->irq_uninstall = gen8_irq_uninstall;
> >> dev->driver->enable_vblank = gen8_enable_vblank;
> >> dev->driver->disable_vblank = gen8_disable_vblank;
> >> - if (HAS_PCH_SPLIT(dev))
> >> - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> >> - else
> >> + if (IS_BROXTON(dev))
> >> dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
> >> + else if (HAS_PCH_SPT(dev))
> >> + dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
> >> + else
> >> + dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> >> } else if (HAS_PCH_SPLIT(dev)) {
> >> dev->driver->irq_handler = ironlake_irq_handler;
> >> dev->driver->irq_preinstall = ironlake_irq_reset;
> >> --
> >> 2.4.6
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> >
> > --
> > Paulo Zanoni
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 06/11] drm/i915: Introduce spt_irq_handler()
2015-08-27 16:13 ` Ville Syrjälä
@ 2015-08-27 17:52 ` Ville Syrjälä
0 siblings, 0 replies; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-27 17:52 UTC (permalink / raw)
To: Jani Nikula; +Cc: Intel Graphics Development
On Thu, Aug 27, 2015 at 07:13:49PM +0300, Ville Syrjälä wrote:
> On Thu, Aug 27, 2015 at 10:38:25AM +0300, Jani Nikula wrote:
> > On Thu, 27 Aug 2015, Paulo Zanoni <przanoni@gmail.com> wrote:
> > > 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>
> > >> Starting from SPT the only interrupts living in the south are GMBUS and
> > >> HPD. What's worse some of the SPT specific new bits conflict with some
> > >> other bits on earlier PCH generations. So better not use the
> > >> cpt_irq_handler() for SPT+ anymore.
> > >>
> > >> Also kill the hand rolled port E handling with something more
> > >> standardish. This also avoids accidentally confusing port B and port E
> > >> long pulses since the bits occupy the same positions, just in different
> > >> registers.
> > >>
> > >> Also add a comment noting that the short pulse duration bits are
> > >> reserved on LPT+. The 2ms value we program is 0, so no issue wrt. the
> > >> MBZ in the spec.
> > >>
> > >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> ---
> > >> drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++++++++++-------------
> > >> 1 file changed, 83 insertions(+), 40 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > >> index d12106c..e2485bd 100644
> > >> --- a/drivers/gpu/drm/i915/i915_irq.c
> > >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> > >> @@ -1260,6 +1260,16 @@ static bool bxt_port_hotplug_long_detect(enum port port, u32 val)
> > >> }
> > >> }
> > >>
> > >> +static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
> > >> +{
> > >> + switch (port) {
> > >> + case PORT_E:
> > >> + return val & PORTE_HOTPLUG_LONG_DETECT;
> > >> + default:
> > >> + return false;
> > >> + }
> > >> +}
> > >> +
> > >> static bool pch_port_hotplug_long_detect(enum port port, u32 val)
> > >> {
> > >> switch (port) {
> > >> @@ -1269,8 +1279,6 @@ static bool pch_port_hotplug_long_detect(enum port port, u32 val)
> > >> return val & PORTC_HOTPLUG_LONG_DETECT;
> > >> case PORT_D:
> > >> return val & PORTD_HOTPLUG_LONG_DETECT;
> > >> - case PORT_E:
> > >> - return val & PORTE_HOTPLUG_LONG_DETECT;
> > >> default:
> > >> return false;
> > >> }
> > >> @@ -1771,12 +1779,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> > >> {
> > >> struct drm_i915_private *dev_priv = dev->dev_private;
> > >> int pipe;
> > >> - u32 hotplug_trigger;
> > >> -
> > >> - if (HAS_PCH_SPT(dev))
> > >> - hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT;
> > >> - else
> > >> - hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
> > >> + u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
> > >>
> > >> if (hotplug_trigger) {
> > >> u32 dig_hotplug_reg, pin_mask, long_mask;
> > >> @@ -1784,22 +1787,10 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> > >> dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> > >> I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> > >>
> > >> - if (HAS_PCH_SPT(dev)) {
> > >> - intel_get_hpd_pins(&pin_mask, &long_mask,
> > >> - hotplug_trigger,
> > >> - dig_hotplug_reg, hpd_spt,
> > >> - pch_port_hotplug_long_detect);
> > >> -
> > >> - /* detect PORTE HP event */
> > >> - dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
> > >> - if (pch_port_hotplug_long_detect(PORT_E,
> > >> - dig_hotplug_reg))
> > >> - long_mask |= 1 << HPD_PORT_E;
> > >> - } else
> > >> - intel_get_hpd_pins(&pin_mask, &long_mask,
> > >> - hotplug_trigger,
> > >> - dig_hotplug_reg, hpd_cpt,
> > >> - pch_port_hotplug_long_detect);
> > >> + intel_get_hpd_pins(&pin_mask, &long_mask,
> > >> + hotplug_trigger,
> > >> + dig_hotplug_reg, hpd_cpt,
> > >> + pch_port_hotplug_long_detect);
> > >>
> > >> intel_hpd_irq_handler(dev, pin_mask, long_mask);
> > >> }
> > >> @@ -1833,6 +1824,42 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> > >> cpt_serr_int_handler(dev);
> > >> }
> > >>
> > >> +static void spt_irq_handler(struct drm_device *dev, u32 pch_iir)
> > >> +{
> > >> + struct drm_i915_private *dev_priv = dev->dev_private;
> > >> + u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
> > >> + ~SDE_PORTE_HOTPLUG_SPT;
> > >> + u32 hotplug2_trigger = pch_iir & SDE_PORTE_HOTPLUG_SPT;
> > >> +
> > >> + if (hotplug_trigger) {
> > >> + u32 dig_hotplug_reg, pin_mask, long_mask;
> > >> +
> > >> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> > >> + I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> > >> +
> > >> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > >> + dig_hotplug_reg, hpd_spt,
> > >> + pch_port_hotplug_long_detect);
> > >> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> > >> + }
> > >> +
> > >> + if (hotplug2_trigger) {
> > >> + u32 dig_hotplug_reg, pin_mask, long_mask;
> > >> +
> > >> + dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
> > >> + I915_WRITE(PCH_PORT_HOTPLUG2, dig_hotplug_reg);
> > >> +
> > >> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug2_trigger,
> > >> + dig_hotplug_reg, hpd_spt,
> > >> + spt_port_hotplug2_long_detect);
> > >> +
> > >> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> > >> + }
> > >
> > > Instead of calling intel_hpd_irq_handler() twice, can't you just do
> > > something like:
> > > intel_hpd_irq_hanlder(dev, pin_mask1 | pin_mask2, long_mask1 | long_mask2) ?
> >
> >
> > And if you reverted
> >
> > commit fd63e2a972c670887e5e8a08440111d3812c0996
> > Author: Imre Deak <imre.deak@intel.com>
> > Date: Tue Jul 21 15:32:44 2015 -0700
> >
> > drm/i915: combine i9xx_get_hpd_pins and pch_get_hpd_pins
> >
> > and added another version for spt, this would all be pretty and each of
> > the functions would do exactly what is required and you could have
> > meaningful, platform specific debug logging in each of the functions.
> >
> > By focusing on deduplicating a few lines in get_hpd_pins, the callers
> > (which are platform specific code to begin with) get more complicated.
>
> OK, let me have a look at this mess in a bit more detail and see what can
> be done. Paulo's idea at least is simple enough to do, so I'll do at
> least that.
I think what I'll do is leave intel_get_hpd_pins() mostly intact, except
I'll move the pin_mask/long_mask 0 initialization out into the callers.
That way anyone can call it multiple times and accumulate all the bits to
the same two masks, and then call intel_hpd_irq_handler just once.
Going back to platform specific variants of the function didn't seem too
nice as I'd probably then need to add more variants for port A HPD etc.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 07/11] drm/i915: Add port A HPD support for ILK/SNB
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (5 preceding siblings ...)
2015-08-12 15:44 ` [PATCH 06/11] drm/i915: Introduce spt_irq_handler() ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-27 18:24 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 08/11] drm/i915: Add port A HPD support for IVB/HSW ville.syrjala
` (5 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
ILK/SNB support port A HPD. While HPD is optional on eDP let's at least
try to wite it up so that we might notice if the link has issues.
The eDP spec suggests that if HPD is not wired up, one should poll the
link status instead. We don't even do that currently.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 59 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 56 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e2485bd..152be8b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -45,6 +45,10 @@
* and related files, but that will be described in separate chapters.
*/
+static const u32 hpd_ilk[HPD_NUM_PINS] = {
+ [HPD_PORT_A] = DE_DP_A_HOTPLUG,
+};
+
static const u32 hpd_ibx[HPD_NUM_PINS] = {
[HPD_CRT] = SDE_CRT_HOTPLUG,
[HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
@@ -1270,6 +1274,16 @@ static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
}
}
+static bool ilk_port_hotplug_long_detect(enum port port, u32 val)
+{
+ switch (port) {
+ case PORT_A:
+ return val & DIGITAL_PORTA_HOTPLUG_LONG_DETECT;
+ default:
+ return false;
+ }
+}
+
static bool pch_port_hotplug_long_detect(enum port port, u32 val)
{
switch (port) {
@@ -1864,6 +1878,19 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
{
struct drm_i915_private *dev_priv = dev->dev_private;
enum pipe pipe;
+ u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG;
+
+ if (hotplug_trigger) {
+ u32 dig_hotplug_reg, pin_mask, long_mask;
+
+ dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
+ I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
+
+ intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+ dig_hotplug_reg, hpd_ilk,
+ ilk_port_hotplug_long_detect);
+ intel_hpd_irq_handler(dev, pin_mask, long_mask);
+ }
if (de_iir & DE_AUX_CHANNEL_A)
dp_aux_irq_handler(dev);
@@ -3105,6 +3132,28 @@ static void spt_hpd_irq_setup(struct drm_device *dev)
I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
}
+static void ilk_hpd_irq_setup(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 hotplug_irqs, hotplug, enabled_irqs;
+
+ hotplug_irqs = DE_DP_A_HOTPLUG;
+ enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
+
+ ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
+
+ /*
+ * Enable digital hotplug on the CPU, and configure the DP short pulse
+ * duration to 2ms (which is the minimum in the Display Port spec)
+ */
+ hotplug = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
+ hotplug &= ~DIGITAL_PORTA_PULSE_DURATION_MASK;
+ hotplug |= DIGITAL_PORTA_HOTPLUG_ENABLE | DIGITAL_PORTA_PULSE_DURATION_2ms;
+ I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, hotplug);
+
+ ibx_hpd_irq_setup(dev);
+}
+
static void bxt_hpd_irq_setup(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3203,8 +3252,9 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
DE_AUX_CHANNEL_A |
DE_PIPEB_CRC_DONE | DE_PIPEA_CRC_DONE |
DE_POISON);
- extra_mask = DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT |
- DE_PIPEB_FIFO_UNDERRUN | DE_PIPEA_FIFO_UNDERRUN;
+ extra_mask = (DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT |
+ DE_PIPEB_FIFO_UNDERRUN | DE_PIPEA_FIFO_UNDERRUN |
+ DE_DP_A_HOTPLUG);
}
dev_priv->irq_mask = ~display_mask;
@@ -4220,7 +4270,10 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
dev->driver->irq_uninstall = ironlake_irq_uninstall;
dev->driver->enable_vblank = ironlake_enable_vblank;
dev->driver->disable_vblank = ironlake_disable_vblank;
- dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
+ if (INTEL_INFO(dev)->gen >= 7)
+ dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
+ else
+ dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
} else {
if (INTEL_INFO(dev_priv)->gen == 2) {
dev->driver->irq_preinstall = i8xx_irq_preinstall;
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 07/11] drm/i915: Add port A HPD support for ILK/SNB
2015-08-12 15:44 ` [PATCH 07/11] drm/i915: Add port A HPD support for ILK/SNB ville.syrjala
@ 2015-08-27 18:24 ` Paulo Zanoni
0 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-27 18:24 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> ILK/SNB support port A HPD. While HPD is optional on eDP let's at least
> try to wite it up so that we might notice if the link has issues.
>
> The eDP spec suggests that if HPD is not wired up, one should poll the
> link status instead. We don't even do that currently.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 59 ++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index e2485bd..152be8b 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -45,6 +45,10 @@
> * and related files, but that will be described in separate chapters.
> */
>
> +static const u32 hpd_ilk[HPD_NUM_PINS] = {
> + [HPD_PORT_A] = DE_DP_A_HOTPLUG,
> +};
> +
> static const u32 hpd_ibx[HPD_NUM_PINS] = {
> [HPD_CRT] = SDE_CRT_HOTPLUG,
> [HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
> @@ -1270,6 +1274,16 @@ static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
> }
> }
>
> +static bool ilk_port_hotplug_long_detect(enum port port, u32 val)
> +{
> + switch (port) {
> + case PORT_A:
> + return val & DIGITAL_PORTA_HOTPLUG_LONG_DETECT;
> + default:
> + return false;
> + }
> +}
> +
> static bool pch_port_hotplug_long_detect(enum port port, u32 val)
> {
> switch (port) {
> @@ -1864,6 +1878,19 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> enum pipe pipe;
> + u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG;
> +
> + if (hotplug_trigger) {
> + u32 dig_hotplug_reg, pin_mask, long_mask;
> +
> + dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
> + I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
> +
> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> + dig_hotplug_reg, hpd_ilk,
> + ilk_port_hotplug_long_detect);
> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> + }
>
> if (de_iir & DE_AUX_CHANNEL_A)
> dp_aux_irq_handler(dev);
> @@ -3105,6 +3132,28 @@ static void spt_hpd_irq_setup(struct drm_device *dev)
> I915_WRITE(PCH_PORT_HOTPLUG2, hotplug);
> }
>
> +static void ilk_hpd_irq_setup(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 hotplug_irqs, hotplug, enabled_irqs;
> +
> + hotplug_irqs = DE_DP_A_HOTPLUG;
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
> +
> + ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
> +
> + /*
> + * Enable digital hotplug on the CPU, and configure the DP short pulse
> + * duration to 2ms (which is the minimum in the Display Port spec)
> + */
> + hotplug = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
> + hotplug &= ~DIGITAL_PORTA_PULSE_DURATION_MASK;
> + hotplug |= DIGITAL_PORTA_HOTPLUG_ENABLE | DIGITAL_PORTA_PULSE_DURATION_2ms;
> + I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, hotplug);
> +
> + ibx_hpd_irq_setup(dev);
> +}
> +
> static void bxt_hpd_irq_setup(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3203,8 +3252,9 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
> DE_AUX_CHANNEL_A |
> DE_PIPEB_CRC_DONE | DE_PIPEA_CRC_DONE |
> DE_POISON);
> - extra_mask = DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT |
> - DE_PIPEB_FIFO_UNDERRUN | DE_PIPEA_FIFO_UNDERRUN;
> + extra_mask = (DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT |
> + DE_PIPEB_FIFO_UNDERRUN | DE_PIPEA_FIFO_UNDERRUN |
> + DE_DP_A_HOTPLUG);
> }
>
> dev_priv->irq_mask = ~display_mask;
> @@ -4220,7 +4270,10 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> dev->driver->irq_uninstall = ironlake_irq_uninstall;
> dev->driver->enable_vblank = ironlake_enable_vblank;
> dev->driver->disable_vblank = ironlake_disable_vblank;
> - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> + if (INTEL_INFO(dev)->gen >= 7)
> + dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> + else
> + dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> } else {
> if (INTEL_INFO(dev_priv)->gen == 2) {
> dev->driver->irq_preinstall = i8xx_irq_preinstall;
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 08/11] drm/i915: Add port A HPD support for IVB/HSW
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (6 preceding siblings ...)
2015-08-12 15:44 ` [PATCH 07/11] drm/i915: Add port A HPD support for ILK/SNB ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-14 9:17 ` Daniel Vetter
2015-08-27 18:30 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 09/11] drm/i915: LPT:LP needs port A HPD enabled in both north and south ville.syrjala
` (4 subsequent siblings)
12 siblings, 2 replies; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
As with ILK/SNB wire up the port A HPD on IVB/HSW.
This might be more important on HSW with PSR. BSpec tells us that if the
automagic link training performed by the hardware fails for some reason,
we're going to get a short HPD and are supposed to re-train the link
manyally.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 152be8b..d994b80 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -49,6 +49,10 @@ static const u32 hpd_ilk[HPD_NUM_PINS] = {
[HPD_PORT_A] = DE_DP_A_HOTPLUG,
};
+static const u32 hpd_ivb[HPD_NUM_PINS] = {
+ [HPD_PORT_A] = DE_DP_A_HOTPLUG_IVB,
+};
+
static const u32 hpd_ibx[HPD_NUM_PINS] = {
[HPD_CRT] = SDE_CRT_HOTPLUG,
[HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
@@ -1940,6 +1944,19 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
{
struct drm_i915_private *dev_priv = dev->dev_private;
enum pipe pipe;
+ u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG_IVB;
+
+ if (hotplug_trigger) {
+ u32 dig_hotplug_reg, pin_mask, long_mask;
+
+ dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
+ I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
+
+ intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+ dig_hotplug_reg, hpd_ivb,
+ ilk_port_hotplug_long_detect);
+ intel_hpd_irq_handler(dev, pin_mask, long_mask);
+ }
if (de_iir & DE_ERR_INT_IVB)
ivb_err_int_handler(dev);
@@ -3137,8 +3154,13 @@ static void ilk_hpd_irq_setup(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
u32 hotplug_irqs, hotplug, enabled_irqs;
- hotplug_irqs = DE_DP_A_HOTPLUG;
- enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
+ if (INTEL_INFO(dev)->gen >= 7) {
+ hotplug_irqs = DE_DP_A_HOTPLUG_IVB;
+ enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ivb);
+ } else {
+ hotplug_irqs = DE_DP_A_HOTPLUG;
+ enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
+ }
ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
@@ -3245,7 +3267,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
DE_PLANEB_FLIP_DONE_IVB |
DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
- DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB);
+ DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB |
+ DE_DP_A_HOTPLUG_IVB);
} else {
display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
@@ -4270,10 +4293,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
dev->driver->irq_uninstall = ironlake_irq_uninstall;
dev->driver->enable_vblank = ironlake_enable_vblank;
dev->driver->disable_vblank = ironlake_disable_vblank;
- if (INTEL_INFO(dev)->gen >= 7)
- dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
- else
- dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
+ dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
} else {
if (INTEL_INFO(dev_priv)->gen == 2) {
dev->driver->irq_preinstall = i8xx_irq_preinstall;
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 08/11] drm/i915: Add port A HPD support for IVB/HSW
2015-08-12 15:44 ` [PATCH 08/11] drm/i915: Add port A HPD support for IVB/HSW ville.syrjala
@ 2015-08-14 9:17 ` Daniel Vetter
2015-08-27 18:30 ` Paulo Zanoni
1 sibling, 0 replies; 41+ messages in thread
From: Daniel Vetter @ 2015-08-14 9:17 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Wed, Aug 12, 2015 at 06:44:17PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> As with ILK/SNB wire up the port A HPD on IVB/HSW.
>
> This might be more important on HSW with PSR. BSpec tells us that if the
> automagic link training performed by the hardware fails for some reason,
> we're going to get a short HPD and are supposed to re-train the link
> manyally.
Rodrigo, could this be the cause behind our frozen screens? If we get out
of PSR and the hw link train fails then the screen will indeed freeze ...
-Daniel
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 34 +++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 152be8b..d994b80 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -49,6 +49,10 @@ static const u32 hpd_ilk[HPD_NUM_PINS] = {
> [HPD_PORT_A] = DE_DP_A_HOTPLUG,
> };
>
> +static const u32 hpd_ivb[HPD_NUM_PINS] = {
> + [HPD_PORT_A] = DE_DP_A_HOTPLUG_IVB,
> +};
> +
> static const u32 hpd_ibx[HPD_NUM_PINS] = {
> [HPD_CRT] = SDE_CRT_HOTPLUG,
> [HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
> @@ -1940,6 +1944,19 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> enum pipe pipe;
> + u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG_IVB;
> +
> + if (hotplug_trigger) {
> + u32 dig_hotplug_reg, pin_mask, long_mask;
> +
> + dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
> + I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
> +
> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> + dig_hotplug_reg, hpd_ivb,
> + ilk_port_hotplug_long_detect);
> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> + }
>
> if (de_iir & DE_ERR_INT_IVB)
> ivb_err_int_handler(dev);
> @@ -3137,8 +3154,13 @@ static void ilk_hpd_irq_setup(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 hotplug_irqs, hotplug, enabled_irqs;
>
> - hotplug_irqs = DE_DP_A_HOTPLUG;
> - enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
> + if (INTEL_INFO(dev)->gen >= 7) {
> + hotplug_irqs = DE_DP_A_HOTPLUG_IVB;
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ivb);
> + } else {
> + hotplug_irqs = DE_DP_A_HOTPLUG;
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
> + }
>
> ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
>
> @@ -3245,7 +3267,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
> DE_PLANEB_FLIP_DONE_IVB |
> DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
> extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
> - DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB);
> + DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB |
> + DE_DP_A_HOTPLUG_IVB);
> } else {
> display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
> DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
> @@ -4270,10 +4293,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> dev->driver->irq_uninstall = ironlake_irq_uninstall;
> dev->driver->enable_vblank = ironlake_enable_vblank;
> dev->driver->disable_vblank = ironlake_disable_vblank;
> - if (INTEL_INFO(dev)->gen >= 7)
> - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> - else
> - dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> + dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> } else {
> if (INTEL_INFO(dev_priv)->gen == 2) {
> dev->driver->irq_preinstall = i8xx_irq_preinstall;
> --
> 2.4.6
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 08/11] drm/i915: Add port A HPD support for IVB/HSW
2015-08-12 15:44 ` [PATCH 08/11] drm/i915: Add port A HPD support for IVB/HSW ville.syrjala
2015-08-14 9:17 ` Daniel Vetter
@ 2015-08-27 18:30 ` Paulo Zanoni
1 sibling, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-27 18:30 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> As with ILK/SNB wire up the port A HPD on IVB/HSW.
>
> This might be more important on HSW with PSR. BSpec tells us that if the
> automagic link training performed by the hardware fails for some reason,
> we're going to get a short HPD and are supposed to re-train the link
> manyally.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 34 +++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 152be8b..d994b80 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -49,6 +49,10 @@ static const u32 hpd_ilk[HPD_NUM_PINS] = {
> [HPD_PORT_A] = DE_DP_A_HOTPLUG,
> };
>
> +static const u32 hpd_ivb[HPD_NUM_PINS] = {
> + [HPD_PORT_A] = DE_DP_A_HOTPLUG_IVB,
> +};
> +
> static const u32 hpd_ibx[HPD_NUM_PINS] = {
> [HPD_CRT] = SDE_CRT_HOTPLUG,
> [HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
> @@ -1940,6 +1944,19 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> enum pipe pipe;
> + u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG_IVB;
> +
> + if (hotplug_trigger) {
> + u32 dig_hotplug_reg, pin_mask, long_mask;
> +
> + dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
> + I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
> +
> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> + dig_hotplug_reg, hpd_ivb,
> + ilk_port_hotplug_long_detect);
> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> + }
>
> if (de_iir & DE_ERR_INT_IVB)
> ivb_err_int_handler(dev);
> @@ -3137,8 +3154,13 @@ static void ilk_hpd_irq_setup(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 hotplug_irqs, hotplug, enabled_irqs;
>
> - hotplug_irqs = DE_DP_A_HOTPLUG;
> - enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
> + if (INTEL_INFO(dev)->gen >= 7) {
> + hotplug_irqs = DE_DP_A_HOTPLUG_IVB;
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ivb);
> + } else {
> + hotplug_irqs = DE_DP_A_HOTPLUG;
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
> + }
>
> ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
>
> @@ -3245,7 +3267,8 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
> DE_PLANEB_FLIP_DONE_IVB |
> DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
> extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
> - DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB);
> + DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB |
> + DE_DP_A_HOTPLUG_IVB);
> } else {
> display_mask = (DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
> DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
> @@ -4270,10 +4293,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> dev->driver->irq_uninstall = ironlake_irq_uninstall;
> dev->driver->enable_vblank = ironlake_enable_vblank;
> dev->driver->disable_vblank = ironlake_disable_vblank;
> - if (INTEL_INFO(dev)->gen >= 7)
> - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> - else
> - dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> + dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> } else {
> if (INTEL_INFO(dev_priv)->gen == 2) {
> dev->driver->irq_preinstall = i8xx_irq_preinstall;
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 09/11] drm/i915: LPT:LP needs port A HPD enabled in both north and south
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (7 preceding siblings ...)
2015-08-12 15:44 ` [PATCH 08/11] drm/i915: Add port A HPD support for IVB/HSW ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-27 18:40 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 10/11] drm/i915: Add port A HPD support for BDW ville.syrjala
` (3 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Supposedly we have to enable port A HPD also in the south on LPT:LP.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d994b80..de60174 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3125,6 +3125,8 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
hotplug |= PORTD_HOTPLUG_ENABLE | PORTD_PULSE_DURATION_2ms;
hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
+ if (HAS_PCH_LPT_LP(dev))
+ hotplug |= PORTA_HOTPLUG_ENABLE;
I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
}
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 09/11] drm/i915: LPT:LP needs port A HPD enabled in both north and south
2015-08-12 15:44 ` [PATCH 09/11] drm/i915: LPT:LP needs port A HPD enabled in both north and south ville.syrjala
@ 2015-08-27 18:40 ` Paulo Zanoni
0 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-27 18:40 UTC (permalink / raw)
To: Ville Syrjälä, Arthur Ranyan; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Supposedly we have to enable port A HPD also in the south on LPT:LP.
The correctness of this patch is going to be decided by the discussion
of patch 5, and up to this moment the answer we have is not 100%
precise.
If (conclusion == "CPU and PCH are in the same package _if and only
if_ HAS_PCH_LPT_LP")
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d994b80..de60174 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3125,6 +3125,8 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
> hotplug |= PORTD_HOTPLUG_ENABLE | PORTD_PULSE_DURATION_2ms;
> hotplug |= PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_2ms;
> hotplug |= PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_2ms;
> + if (HAS_PCH_LPT_LP(dev))
> + hotplug |= PORTA_HOTPLUG_ENABLE;
> I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
> }
>
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 10/11] drm/i915: Add port A HPD support for BDW
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (8 preceding siblings ...)
2015-08-12 15:44 ` [PATCH 09/11] drm/i915: LPT:LP needs port A HPD enabled in both north and south ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-27 19:29 ` Paulo Zanoni
2015-08-12 15:44 ` [PATCH 11/11] drm/i915: Add port A HPD support for SPT ville.syrjala
` (2 subsequent siblings)
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Wire up the port A HPD for BDW. Compared to earlier platforms the
interrupt setup is a bit different, but basically everything else
looks the same.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 72 +++++++++++++++++++++++++++++++++++++----
1 file changed, 66 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index de60174..aefa6c4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -53,6 +53,10 @@ static const u32 hpd_ivb[HPD_NUM_PINS] = {
[HPD_PORT_A] = DE_DP_A_HOTPLUG_IVB,
};
+static const u32 hpd_bdw[HPD_NUM_PINS] = {
+ [HPD_PORT_A] = GEN8_PORT_DP_A_HOTPLUG,
+};
+
static const u32 hpd_ibx[HPD_NUM_PINS] = {
[HPD_CRT] = SDE_CRT_HOTPLUG,
[HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
@@ -369,6 +373,36 @@ void gen6_disable_rps_interrupts(struct drm_device *dev)
}
/**
+ * bdw_update_port_irq - update DE port interrupt
+ * @dev_priv: driver private
+ * @interrupt_mask: mask of interrupt bits to update
+ * @enabled_irq_mask: mask of interrupt bits to enable
+ */
+static void bdw_update_port_irq(struct drm_i915_private *dev_priv,
+ uint32_t interrupt_mask,
+ uint32_t enabled_irq_mask)
+{
+ uint32_t new_val;
+ uint32_t old_val;
+
+ assert_spin_locked(&dev_priv->irq_lock);
+
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
+ return;
+
+ old_val = I915_READ(GEN8_DE_PORT_IMR);
+
+ new_val = old_val;
+ new_val &= ~interrupt_mask;
+ new_val |= (~enabled_irq_mask & interrupt_mask);
+
+ if (new_val != old_val) {
+ I915_WRITE(GEN8_DE_PORT_IMR, new_val);
+ POSTING_READ(GEN8_DE_PORT_IMR);
+ }
+}
+
+/**
* ibx_display_interrupt_update - update SDEIMR
* @dev_priv: driver private
* @interrupt_mask: mask of interrupt bits to update
@@ -2139,10 +2173,23 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
tmp = I915_READ(GEN8_DE_PORT_IIR);
if (tmp) {
bool found = false;
+ u32 hotplug_trigger = tmp & GEN8_PORT_DP_A_HOTPLUG;
I915_WRITE(GEN8_DE_PORT_IIR, tmp);
ret = IRQ_HANDLED;
+ if (hotplug_trigger) {
+ u32 dig_hotplug_reg, pin_mask, long_mask;
+
+ dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
+ I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
+
+ intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
+ dig_hotplug_reg, hpd_bdw,
+ ilk_port_hotplug_long_detect);
+ intel_hpd_irq_handler(dev, pin_mask, long_mask);
+ }
+
if (tmp & aux_mask) {
dp_aux_irq_handler(dev);
found = true;
@@ -3156,15 +3203,22 @@ static void ilk_hpd_irq_setup(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
u32 hotplug_irqs, hotplug, enabled_irqs;
- if (INTEL_INFO(dev)->gen >= 7) {
+ if (INTEL_INFO(dev)->gen >= 8) {
+ hotplug_irqs = GEN8_PORT_DP_A_HOTPLUG;
+ enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_bdw);
+
+ bdw_update_port_irq(dev_priv, hotplug_irqs, enabled_irqs);
+ } else if (INTEL_INFO(dev)->gen >= 7) {
hotplug_irqs = DE_DP_A_HOTPLUG_IVB;
enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ivb);
+
+ ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
} else {
hotplug_irqs = DE_DP_A_HOTPLUG;
enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
- }
- ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
+ ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
+ }
/*
* Enable digital hotplug on the CPU, and configure the DP short pulse
@@ -3477,6 +3531,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
uint32_t de_pipe_enables;
int pipe;
u32 de_port_en = GEN8_AUX_CHANNEL_A;
+ u32 de_port_masked;
if (IS_GEN9(dev_priv)) {
de_pipe_masked |= GEN9_PIPE_PLANE1_FLIP_DONE |
@@ -3486,9 +3541,14 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
if (IS_BROXTON(dev_priv))
de_port_en |= BXT_DE_PORT_GMBUS;
- } else
+ } else {
de_pipe_masked |= GEN8_PIPE_PRIMARY_FLIP_DONE |
GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
+ }
+
+ de_port_masked = de_port_en;
+ if (IS_BROADWELL(dev_priv))
+ de_port_masked |= GEN8_PORT_DP_A_HOTPLUG;
de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
GEN8_PIPE_FIFO_UNDERRUN;
@@ -3504,7 +3564,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
dev_priv->de_irq_mask[pipe],
de_pipe_enables);
- GEN5_IRQ_INIT(GEN8_DE_PORT_, ~de_port_en, de_port_en);
+ GEN5_IRQ_INIT(GEN8_DE_PORT_, ~de_port_en, de_port_masked);
}
static int gen8_irq_postinstall(struct drm_device *dev)
@@ -4287,7 +4347,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
else if (HAS_PCH_SPT(dev))
dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
else
- dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
+ dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
} else if (HAS_PCH_SPLIT(dev)) {
dev->driver->irq_handler = ironlake_irq_handler;
dev->driver->irq_preinstall = ironlake_irq_reset;
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 10/11] drm/i915: Add port A HPD support for BDW
2015-08-12 15:44 ` [PATCH 10/11] drm/i915: Add port A HPD support for BDW ville.syrjala
@ 2015-08-27 19:29 ` Paulo Zanoni
2015-08-27 19:51 ` Ville Syrjälä
0 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-27 19:29 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Wire up the port A HPD for BDW. Compared to earlier platforms the
> interrupt setup is a bit different, but basically everything else
> looks the same.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 72 +++++++++++++++++++++++++++++++++++++----
> 1 file changed, 66 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index de60174..aefa6c4 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -53,6 +53,10 @@ static const u32 hpd_ivb[HPD_NUM_PINS] = {
> [HPD_PORT_A] = DE_DP_A_HOTPLUG_IVB,
> };
>
> +static const u32 hpd_bdw[HPD_NUM_PINS] = {
> + [HPD_PORT_A] = GEN8_PORT_DP_A_HOTPLUG,
> +};
> +
> static const u32 hpd_ibx[HPD_NUM_PINS] = {
> [HPD_CRT] = SDE_CRT_HOTPLUG,
> [HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
> @@ -369,6 +373,36 @@ void gen6_disable_rps_interrupts(struct drm_device *dev)
> }
>
> /**
> + * bdw_update_port_irq - update DE port interrupt
> + * @dev_priv: driver private
> + * @interrupt_mask: mask of interrupt bits to update
> + * @enabled_irq_mask: mask of interrupt bits to enable
> + */
> +static void bdw_update_port_irq(struct drm_i915_private *dev_priv,
> + uint32_t interrupt_mask,
> + uint32_t enabled_irq_mask)
> +{
> + uint32_t new_val;
> + uint32_t old_val;
> +
> + assert_spin_locked(&dev_priv->irq_lock);
Just like the other similar functions:
WARN_ON(enabled_irq_mask & ~interrupt_mask);
Besides this, there's the recurring "enable PORT A hpd on the PCH"
problem. Don't you have to patch ibx_hpd_irq_setup() to only enable
PORTA_HOTPLUG_ENABLE based on what you read from FUSE_STRAP3? At least
that's what's written on the description of 0x44030 on BDW.
Everything else looks correct.
> +
> + if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> + return;
> +
> + old_val = I915_READ(GEN8_DE_PORT_IMR);
> +
> + new_val = old_val;
> + new_val &= ~interrupt_mask;
> + new_val |= (~enabled_irq_mask & interrupt_mask);
> +
> + if (new_val != old_val) {
> + I915_WRITE(GEN8_DE_PORT_IMR, new_val);
> + POSTING_READ(GEN8_DE_PORT_IMR);
> + }
> +}
> +
> +/**
> * ibx_display_interrupt_update - update SDEIMR
> * @dev_priv: driver private
> * @interrupt_mask: mask of interrupt bits to update
> @@ -2139,10 +2173,23 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
> tmp = I915_READ(GEN8_DE_PORT_IIR);
> if (tmp) {
> bool found = false;
> + u32 hotplug_trigger = tmp & GEN8_PORT_DP_A_HOTPLUG;
>
> I915_WRITE(GEN8_DE_PORT_IIR, tmp);
> ret = IRQ_HANDLED;
>
> + if (hotplug_trigger) {
> + u32 dig_hotplug_reg, pin_mask, long_mask;
> +
> + dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
> + I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
> +
> + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> + dig_hotplug_reg, hpd_bdw,
> + ilk_port_hotplug_long_detect);
> + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> + }
> +
> if (tmp & aux_mask) {
> dp_aux_irq_handler(dev);
> found = true;
> @@ -3156,15 +3203,22 @@ static void ilk_hpd_irq_setup(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 hotplug_irqs, hotplug, enabled_irqs;
>
> - if (INTEL_INFO(dev)->gen >= 7) {
> + if (INTEL_INFO(dev)->gen >= 8) {
> + hotplug_irqs = GEN8_PORT_DP_A_HOTPLUG;
> + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_bdw);
> +
> + bdw_update_port_irq(dev_priv, hotplug_irqs, enabled_irqs);
> + } else if (INTEL_INFO(dev)->gen >= 7) {
> hotplug_irqs = DE_DP_A_HOTPLUG_IVB;
> enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ivb);
> +
> + ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
> } else {
> hotplug_irqs = DE_DP_A_HOTPLUG;
> enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
> - }
>
> - ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
> + ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
> + }
>
> /*
> * Enable digital hotplug on the CPU, and configure the DP short pulse
> @@ -3477,6 +3531,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> uint32_t de_pipe_enables;
> int pipe;
> u32 de_port_en = GEN8_AUX_CHANNEL_A;
> + u32 de_port_masked;
>
> if (IS_GEN9(dev_priv)) {
> de_pipe_masked |= GEN9_PIPE_PLANE1_FLIP_DONE |
> @@ -3486,9 +3541,14 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>
> if (IS_BROXTON(dev_priv))
> de_port_en |= BXT_DE_PORT_GMBUS;
> - } else
> + } else {
> de_pipe_masked |= GEN8_PIPE_PRIMARY_FLIP_DONE |
> GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
> + }
> +
> + de_port_masked = de_port_en;
> + if (IS_BROADWELL(dev_priv))
> + de_port_masked |= GEN8_PORT_DP_A_HOTPLUG;
>
> de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> GEN8_PIPE_FIFO_UNDERRUN;
> @@ -3504,7 +3564,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> dev_priv->de_irq_mask[pipe],
> de_pipe_enables);
>
> - GEN5_IRQ_INIT(GEN8_DE_PORT_, ~de_port_en, de_port_en);
> + GEN5_IRQ_INIT(GEN8_DE_PORT_, ~de_port_en, de_port_masked);
> }
>
> static int gen8_irq_postinstall(struct drm_device *dev)
> @@ -4287,7 +4347,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> else if (HAS_PCH_SPT(dev))
> dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
> else
> - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> + dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> } else if (HAS_PCH_SPLIT(dev)) {
> dev->driver->irq_handler = ironlake_irq_handler;
> dev->driver->irq_preinstall = ironlake_irq_reset;
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 10/11] drm/i915: Add port A HPD support for BDW
2015-08-27 19:29 ` Paulo Zanoni
@ 2015-08-27 19:51 ` Ville Syrjälä
0 siblings, 0 replies; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-27 19:51 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development
On Thu, Aug 27, 2015 at 04:29:21PM -0300, Paulo Zanoni wrote:
> 2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Wire up the port A HPD for BDW. Compared to earlier platforms the
> > interrupt setup is a bit different, but basically everything else
> > looks the same.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 72 +++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 66 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index de60174..aefa6c4 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -53,6 +53,10 @@ static const u32 hpd_ivb[HPD_NUM_PINS] = {
> > [HPD_PORT_A] = DE_DP_A_HOTPLUG_IVB,
> > };
> >
> > +static const u32 hpd_bdw[HPD_NUM_PINS] = {
> > + [HPD_PORT_A] = GEN8_PORT_DP_A_HOTPLUG,
> > +};
> > +
> > static const u32 hpd_ibx[HPD_NUM_PINS] = {
> > [HPD_CRT] = SDE_CRT_HOTPLUG,
> > [HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
> > @@ -369,6 +373,36 @@ void gen6_disable_rps_interrupts(struct drm_device *dev)
> > }
> >
> > /**
> > + * bdw_update_port_irq - update DE port interrupt
> > + * @dev_priv: driver private
> > + * @interrupt_mask: mask of interrupt bits to update
> > + * @enabled_irq_mask: mask of interrupt bits to enable
> > + */
> > +static void bdw_update_port_irq(struct drm_i915_private *dev_priv,
> > + uint32_t interrupt_mask,
> > + uint32_t enabled_irq_mask)
> > +{
> > + uint32_t new_val;
> > + uint32_t old_val;
> > +
> > + assert_spin_locked(&dev_priv->irq_lock);
>
> Just like the other similar functions:
> WARN_ON(enabled_irq_mask & ~interrupt_mask);
ack
>
>
> Besides this, there's the recurring "enable PORT A hpd on the PCH"
> problem. Don't you have to patch ibx_hpd_irq_setup() to only enable
> PORTA_HOTPLUG_ENABLE based on what you read from FUSE_STRAP3? At least
> that's what's written on the description of 0x44030 on BDW.
Well, as mentioned in the other mail the strap description says it's not
used and should be ignored, and Art said it might not be correct. So not
sure how we're supposed figure it out if we can't use the LP vs. H
approach. I'll guess I could toss in some FIXMEs or something if we
can't figure it all out soon.
>
> Everything else looks correct.
>
> > +
> > + if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> > + return;
> > +
> > + old_val = I915_READ(GEN8_DE_PORT_IMR);
> > +
> > + new_val = old_val;
> > + new_val &= ~interrupt_mask;
> > + new_val |= (~enabled_irq_mask & interrupt_mask);
> > +
> > + if (new_val != old_val) {
> > + I915_WRITE(GEN8_DE_PORT_IMR, new_val);
> > + POSTING_READ(GEN8_DE_PORT_IMR);
> > + }
> > +}
> > +
> > +/**
> > * ibx_display_interrupt_update - update SDEIMR
> > * @dev_priv: driver private
> > * @interrupt_mask: mask of interrupt bits to update
> > @@ -2139,10 +2173,23 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
> > tmp = I915_READ(GEN8_DE_PORT_IIR);
> > if (tmp) {
> > bool found = false;
> > + u32 hotplug_trigger = tmp & GEN8_PORT_DP_A_HOTPLUG;
> >
> > I915_WRITE(GEN8_DE_PORT_IIR, tmp);
> > ret = IRQ_HANDLED;
> >
> > + if (hotplug_trigger) {
> > + u32 dig_hotplug_reg, pin_mask, long_mask;
> > +
> > + dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
> > + I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
> > +
> > + intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > + dig_hotplug_reg, hpd_bdw,
> > + ilk_port_hotplug_long_detect);
> > + intel_hpd_irq_handler(dev, pin_mask, long_mask);
> > + }
> > +
> > if (tmp & aux_mask) {
> > dp_aux_irq_handler(dev);
> > found = true;
> > @@ -3156,15 +3203,22 @@ static void ilk_hpd_irq_setup(struct drm_device *dev)
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > u32 hotplug_irqs, hotplug, enabled_irqs;
> >
> > - if (INTEL_INFO(dev)->gen >= 7) {
> > + if (INTEL_INFO(dev)->gen >= 8) {
> > + hotplug_irqs = GEN8_PORT_DP_A_HOTPLUG;
> > + enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_bdw);
> > +
> > + bdw_update_port_irq(dev_priv, hotplug_irqs, enabled_irqs);
> > + } else if (INTEL_INFO(dev)->gen >= 7) {
> > hotplug_irqs = DE_DP_A_HOTPLUG_IVB;
> > enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ivb);
> > +
> > + ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
> > } else {
> > hotplug_irqs = DE_DP_A_HOTPLUG;
> > enabled_irqs = intel_hpd_enabled_irqs(dev, hpd_ilk);
> > - }
> >
> > - ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
> > + ilk_update_display_irq(dev_priv, hotplug_irqs, enabled_irqs);
> > + }
> >
> > /*
> > * Enable digital hotplug on the CPU, and configure the DP short pulse
> > @@ -3477,6 +3531,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> > uint32_t de_pipe_enables;
> > int pipe;
> > u32 de_port_en = GEN8_AUX_CHANNEL_A;
> > + u32 de_port_masked;
I think I made a mess with this name. It's exactly the opposite of the
pipe stuff. The resulting code already confused the hell out of me when
I was looking at it the second time. So I think I need to redo this part
a bit.
BTW I'm already cooking up a few extra patches on top of the series,
mainly to get the BXT code to conform to the same style the rest of
the code uses. Since most patches have needed some adjustment I'll
repost the entire series. But I'll wait until you've gone through this
v1 so that I'll get all your feedback incorporated.
> >
> > if (IS_GEN9(dev_priv)) {
> > de_pipe_masked |= GEN9_PIPE_PLANE1_FLIP_DONE |
> > @@ -3486,9 +3541,14 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> >
> > if (IS_BROXTON(dev_priv))
> > de_port_en |= BXT_DE_PORT_GMBUS;
> > - } else
> > + } else {
> > de_pipe_masked |= GEN8_PIPE_PRIMARY_FLIP_DONE |
> > GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
> > + }
> > +
> > + de_port_masked = de_port_en;
> > + if (IS_BROADWELL(dev_priv))
> > + de_port_masked |= GEN8_PORT_DP_A_HOTPLUG;
> >
> > de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> > GEN8_PIPE_FIFO_UNDERRUN;
> > @@ -3504,7 +3564,7 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> > dev_priv->de_irq_mask[pipe],
> > de_pipe_enables);
> >
> > - GEN5_IRQ_INIT(GEN8_DE_PORT_, ~de_port_en, de_port_en);
> > + GEN5_IRQ_INIT(GEN8_DE_PORT_, ~de_port_en, de_port_masked);
> > }
> >
> > static int gen8_irq_postinstall(struct drm_device *dev)
> > @@ -4287,7 +4347,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> > else if (HAS_PCH_SPT(dev))
> > dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
> > else
> > - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> > + dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
> > } else if (HAS_PCH_SPLIT(dev)) {
> > dev->driver->irq_handler = ironlake_irq_handler;
> > dev->driver->irq_preinstall = ironlake_irq_reset;
> > --
> > 2.4.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 11/11] drm/i915: Add port A HPD support for SPT
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (9 preceding siblings ...)
2015-08-12 15:44 ` [PATCH 10/11] drm/i915: Add port A HPD support for BDW ville.syrjala
@ 2015-08-12 15:44 ` ville.syrjala
2015-08-27 20:26 ` Paulo Zanoni
2015-08-19 18:13 ` [PATCH 12/11] drm/i915: Reinitialize HPD after runtime D3 ville.syrjala
2015-08-19 19:11 ` [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups Ville Syrjälä
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-12 15:44 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
On SKL the port A HPD has moved to the PCH. Hook it up.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 21 +++++++++++++++++++--
drivers/gpu/drm/i915/i915_reg.h | 4 +++-
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index aefa6c4..ec739e6 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -74,6 +74,7 @@ static const u32 hpd_cpt[HPD_NUM_PINS] = {
};
static const u32 hpd_spt[HPD_NUM_PINS] = {
+ [HPD_PORT_A] = SDE_PORTA_HOTPLUG_SPT,
[HPD_PORT_B] = SDE_PORTB_HOTPLUG_CPT,
[HPD_PORT_C] = SDE_PORTC_HOTPLUG_CPT,
[HPD_PORT_D] = SDE_PORTD_HOTPLUG_CPT,
@@ -1312,6 +1313,22 @@ static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
}
}
+static bool spt_port_hotplug_long_detect(enum port port, u32 val)
+{
+ switch (port) {
+ case PORT_A:
+ return val & PORTA_HOTPLUG_LONG_DETECT;
+ case PORT_B:
+ return val & PORTB_HOTPLUG_LONG_DETECT;
+ case PORT_C:
+ return val & PORTC_HOTPLUG_LONG_DETECT;
+ case PORT_D:
+ return val & PORTD_HOTPLUG_LONG_DETECT;
+ default:
+ return false;
+ }
+}
+
static bool ilk_port_hotplug_long_detect(enum port port, u32 val)
{
switch (port) {
@@ -1891,7 +1908,7 @@ static void spt_irq_handler(struct drm_device *dev, u32 pch_iir)
intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
dig_hotplug_reg, hpd_spt,
- pch_port_hotplug_long_detect);
+ spt_port_hotplug_long_detect);
intel_hpd_irq_handler(dev, pin_mask, long_mask);
}
@@ -3190,7 +3207,7 @@ static void spt_hpd_irq_setup(struct drm_device *dev)
/* Enable digital hotplug on the PCH */
hotplug = I915_READ(PCH_PORT_HOTPLUG);
hotplug |= PORTD_HOTPLUG_ENABLE | PORTC_HOTPLUG_ENABLE |
- PORTB_HOTPLUG_ENABLE;
+ PORTB_HOTPLUG_ENABLE | PORTA_HOTPLUG_ENABLE;
I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
hotplug = I915_READ(PCH_PORT_HOTPLUG2);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0e9990b..3224c97 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5953,6 +5953,7 @@ enum skl_disp_power_wells {
#define SDE_AUXB_CPT (1 << 25)
#define SDE_AUX_MASK_CPT (7 << 25)
#define SDE_PORTE_HOTPLUG_SPT (1 << 25)
+#define SDE_PORTA_HOTPLUG_SPT (1 << 24)
#define SDE_PORTD_HOTPLUG_CPT (1 << 23)
#define SDE_PORTC_HOTPLUG_CPT (1 << 22)
#define SDE_PORTB_HOTPLUG_CPT (1 << 21)
@@ -5966,7 +5967,8 @@ enum skl_disp_power_wells {
#define SDE_HOTPLUG_MASK_SPT (SDE_PORTE_HOTPLUG_SPT | \
SDE_PORTD_HOTPLUG_CPT | \
SDE_PORTC_HOTPLUG_CPT | \
- SDE_PORTB_HOTPLUG_CPT)
+ SDE_PORTB_HOTPLUG_CPT | \
+ SDE_PORTA_HOTPLUG_SPT)
#define SDE_GMBUS_CPT (1 << 17)
#define SDE_ERROR_CPT (1 << 16)
#define SDE_AUDIO_CP_REQ_C_CPT (1 << 10)
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 11/11] drm/i915: Add port A HPD support for SPT
2015-08-12 15:44 ` [PATCH 11/11] drm/i915: Add port A HPD support for SPT ville.syrjala
@ 2015-08-27 20:26 ` Paulo Zanoni
0 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-27 20:26 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-12 12:44 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> On SKL the port A HPD has moved to the PCH. Hook it up.
Today is Déjà vu day.
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 21 +++++++++++++++++++--
> drivers/gpu/drm/i915/i915_reg.h | 4 +++-
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index aefa6c4..ec739e6 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -74,6 +74,7 @@ static const u32 hpd_cpt[HPD_NUM_PINS] = {
> };
>
> static const u32 hpd_spt[HPD_NUM_PINS] = {
> + [HPD_PORT_A] = SDE_PORTA_HOTPLUG_SPT,
> [HPD_PORT_B] = SDE_PORTB_HOTPLUG_CPT,
> [HPD_PORT_C] = SDE_PORTC_HOTPLUG_CPT,
> [HPD_PORT_D] = SDE_PORTD_HOTPLUG_CPT,
> @@ -1312,6 +1313,22 @@ static bool spt_port_hotplug2_long_detect(enum port port, u32 val)
> }
> }
>
> +static bool spt_port_hotplug_long_detect(enum port port, u32 val)
> +{
> + switch (port) {
> + case PORT_A:
> + return val & PORTA_HOTPLUG_LONG_DETECT;
> + case PORT_B:
> + return val & PORTB_HOTPLUG_LONG_DETECT;
> + case PORT_C:
> + return val & PORTC_HOTPLUG_LONG_DETECT;
> + case PORT_D:
> + return val & PORTD_HOTPLUG_LONG_DETECT;
> + default:
> + return false;
> + }
> +}
> +
> static bool ilk_port_hotplug_long_detect(enum port port, u32 val)
> {
> switch (port) {
> @@ -1891,7 +1908,7 @@ static void spt_irq_handler(struct drm_device *dev, u32 pch_iir)
>
> intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> dig_hotplug_reg, hpd_spt,
> - pch_port_hotplug_long_detect);
> + spt_port_hotplug_long_detect);
> intel_hpd_irq_handler(dev, pin_mask, long_mask);
> }
>
> @@ -3190,7 +3207,7 @@ static void spt_hpd_irq_setup(struct drm_device *dev)
> /* Enable digital hotplug on the PCH */
> hotplug = I915_READ(PCH_PORT_HOTPLUG);
> hotplug |= PORTD_HOTPLUG_ENABLE | PORTC_HOTPLUG_ENABLE |
> - PORTB_HOTPLUG_ENABLE;
> + PORTB_HOTPLUG_ENABLE | PORTA_HOTPLUG_ENABLE;
> I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
>
> hotplug = I915_READ(PCH_PORT_HOTPLUG2);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0e9990b..3224c97 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5953,6 +5953,7 @@ enum skl_disp_power_wells {
> #define SDE_AUXB_CPT (1 << 25)
> #define SDE_AUX_MASK_CPT (7 << 25)
> #define SDE_PORTE_HOTPLUG_SPT (1 << 25)
> +#define SDE_PORTA_HOTPLUG_SPT (1 << 24)
> #define SDE_PORTD_HOTPLUG_CPT (1 << 23)
> #define SDE_PORTC_HOTPLUG_CPT (1 << 22)
> #define SDE_PORTB_HOTPLUG_CPT (1 << 21)
> @@ -5966,7 +5967,8 @@ enum skl_disp_power_wells {
> #define SDE_HOTPLUG_MASK_SPT (SDE_PORTE_HOTPLUG_SPT | \
> SDE_PORTD_HOTPLUG_CPT | \
> SDE_PORTC_HOTPLUG_CPT | \
> - SDE_PORTB_HOTPLUG_CPT)
> + SDE_PORTB_HOTPLUG_CPT | \
> + SDE_PORTA_HOTPLUG_SPT)
> #define SDE_GMBUS_CPT (1 << 17)
> #define SDE_ERROR_CPT (1 << 16)
> #define SDE_AUDIO_CP_REQ_C_CPT (1 << 10)
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 12/11] drm/i915: Reinitialize HPD after runtime D3
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (10 preceding siblings ...)
2015-08-12 15:44 ` [PATCH 11/11] drm/i915: Add port A HPD support for SPT ville.syrjala
@ 2015-08-19 18:13 ` ville.syrjala
2015-08-27 20:36 ` Paulo Zanoni
2015-08-19 19:11 ` [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups Ville Syrjälä
12 siblings, 1 reply; 41+ messages in thread
From: ville.syrjala @ 2015-08-19 18:13 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Runtime suspends disabled all interrupts, so in order to get them back
fully we need to also do the HPD irq setup on runtime resume. Except
on VLV/CHV where the display interrupt initialization is part of the
display power well powerup.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1d88745..4bbd3b7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1549,6 +1549,15 @@ static int intel_runtime_resume(struct device *device)
gen6_update_ring_freq(dev);
intel_runtime_pm_enable_interrupts(dev_priv);
+
+ /*
+ * On VLV/CHV display interrupts are part of the display
+ * power well, so hpd is reinitialized from there. For
+ * everyone else do it here.
+ */
+ if (!IS_VALLEYVIEW(dev_priv))
+ intel_hpd_init(dev_priv);
+
intel_enable_gt_powersave(dev);
if (ret)
--
2.4.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 12/11] drm/i915: Reinitialize HPD after runtime D3
2015-08-19 18:13 ` [PATCH 12/11] drm/i915: Reinitialize HPD after runtime D3 ville.syrjala
@ 2015-08-27 20:36 ` Paulo Zanoni
0 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2015-08-27 20:36 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development
2015-08-19 15:13 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Runtime suspends disabled all interrupts, so in order to get them back
> fully we need to also do the HPD irq setup on runtime resume. Except
> on VLV/CHV where the display interrupt initialization is part of the
> display power well powerup.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1d88745..4bbd3b7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1549,6 +1549,15 @@ static int intel_runtime_resume(struct device *device)
> gen6_update_ring_freq(dev);
>
> intel_runtime_pm_enable_interrupts(dev_priv);
> +
> + /*
> + * On VLV/CHV display interrupts are part of the display
> + * power well, so hpd is reinitialized from there. For
> + * everyone else do it here.
> + */
> + if (!IS_VALLEYVIEW(dev_priv))
> + intel_hpd_init(dev_priv);
Maybe we should put this inside intel_runtime_pm_enable_interrupts()?
Either way: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> +
> intel_enable_gt_powersave(dev);
>
> if (ret)
> --
> 2.4.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups
2015-08-12 15:44 [PATCH 00/11] drm/i915: Port A HPD and other HPD cleanups ville.syrjala
` (11 preceding siblings ...)
2015-08-19 18:13 ` [PATCH 12/11] drm/i915: Reinitialize HPD after runtime D3 ville.syrjala
@ 2015-08-19 19:11 ` Ville Syrjälä
12 siblings, 0 replies; 41+ messages in thread
From: Ville Syrjälä @ 2015-08-19 19:11 UTC (permalink / raw)
To: intel-gfx
On Wed, Aug 12, 2015 at 06:44:09PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I've had a bunch of this stuff sitting in a branch for quite a while, almost a
> year by the looks of the git dates :P I also had port E HPD in there but
> something similar has already landed in the meantime so I just rebased my
> junk on top.
>
> I've only quickly tested the port A HPD on my ILK. Don't have other machines
> with eDP on port A here.
Tested on HSW ULT now too. Appears to work fine, well, after I added the
missing hpd reinit on runtime resume. Patch amended to the series.
> Ville Syrjälä (11):
> drm/i915: Clean up various HPD defines
> drm/i915; Extract intel_hpd_enabled_irqs()
> drm/i915: Factor out ilk_update_display_irq()
> drm/i915: Add HAS_PCH_LPT_LP() macro
> drm/i915: Rename BXT PORTA HPD defines
> drm/i915: Introduce spt_irq_handler()
> drm/i915: Add port A HPD support for ILK/SNB
> drm/i915: Add port A HPD support for IVB/HSW
> drm/i915: LPT:LP needs port A HPD enabled in both north and south
> drm/i915: Add port A HPD support for BDW
> drm/i915: Add port A HPD support for SPT
>
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_irq.c | 367 +++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/i915_reg.h | 82 ++++----
> drivers/gpu/drm/i915/intel_display.c | 13 +-
> drivers/gpu/drm/i915/intel_pm.c | 4 +-
> 5 files changed, 336 insertions(+), 131 deletions(-)
>
> --
> 2.4.6
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread