Re: [Intel-gfx] [PATCH v2 16/19] drm/i915: Remove the per-plaform IIR HPD masking

From: Lucas De Marchi <lucas.demarchi@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 16/19] drm/i915: Remove the per-plaform IIR HPD masking
Date: Tue, 27 Oct 2020 18:00:10 -0700	[thread overview]
Message-ID: <20201028010010.3wyfsm5ygyvpbncb@ldmartin-desk1> (raw)
In-Reply-To: <20201023194429.GE6112@intel.com>

On Fri, Oct 23, 2020 at 10:44:29PM +0300, Ville Syrjälä wrote:
>On Fri, Oct 23, 2020 at 12:29:37PM -0700, Lucas De Marchi wrote:
>> On Fri, Oct 23, 2020 at 04:34:17PM +0300, Ville Syrjälä wrote:
>> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> >We no longer unmask all HPD irqs, so we can drop the ugly per-platform
>> >HPD IIR masking. IMR will prevent unsupported bits from appearing in
>> >IIR.
>> >
>> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> this patch should come after
>> "drm/i915: Enable hpd logic only for ports that are present"
>
>The order should not matter. We already mask out all unwanted bits
>from IIR via IMR, so whether or not the HPD level is visible in ISR
>for some unused pin is irrelevant.

indeed. My R-B stands even more now.

Lucas De Marchi

>
>>
>> >---
>> > drivers/gpu/drm/i915/i915_irq.c | 21 ++-------------------
>> > drivers/gpu/drm/i915/i915_reg.h | 10 ++--------
>> > 2 files changed, 4 insertions(+), 27 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> >index be69c129dd5a..447a52f7701d 100644
>> >--- a/drivers/gpu/drm/i915/i915_irq.c
>> >+++ b/drivers/gpu/drm/i915/i915_irq.c
>> >@@ -1883,27 +1883,10 @@ static void cpt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
>> >
>> > static void icp_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
>> > {
>> >-	u32 ddi_hotplug_trigger, tc_hotplug_trigger;
>> >+	u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
>> >+	u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
>> > 	u32 pin_mask = 0, long_mask = 0;
>> >
>> >-	if (HAS_PCH_TGP(dev_priv)) {
>> >-		ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_TGP;
>> >-		tc_hotplug_trigger = pch_iir & SDE_TC_MASK_TGP;
>> >-	} else if (HAS_PCH_JSP(dev_priv)) {
>> >-		ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_TGP;
>> >-		tc_hotplug_trigger = 0;
>> >-	} else if (HAS_PCH_MCC(dev_priv)) {
>> >-		ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
>> >-		tc_hotplug_trigger = pch_iir & SDE_TC_HOTPLUG_ICP(HPD_PORT_TC1);
>> >-	} else {
>> >-		drm_WARN(&dev_priv->drm, !HAS_PCH_ICP(dev_priv),
>> >-			 "Unrecognized PCH type 0x%x\n",
>> >-			 INTEL_PCH_TYPE(dev_priv));
>> >-
>> >-		ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
>> >-		tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
>> >-	}
>> >-
>> > 	if (ddi_hotplug_trigger) {
>> > 		u32 dig_hotplug_reg;
>> >
>> >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> >index 8f26ab69b366..f15914fbe7c3 100644
>> >--- a/drivers/gpu/drm/i915/i915_reg.h
>> >+++ b/drivers/gpu/drm/i915/i915_reg.h
>> >@@ -8351,16 +8351,10 @@ enum {
>> > #define SDE_GMBUS_ICP			(1 << 23)
>> > #define SDE_TC_HOTPLUG_ICP(hpd_pin)	REG_BIT(24 + _HPD_PIN_TC(hpd_pin))
>> > #define SDE_DDI_HOTPLUG_ICP(hpd_pin)	REG_BIT(16 + _HPD_PIN_DDI(hpd_pin))
>> >-#define SDE_DDI_MASK_ICP		(SDE_DDI_HOTPLUG_ICP(HPD_PORT_B) | \
>> >-					 SDE_DDI_HOTPLUG_ICP(HPD_PORT_A))
>> >-#define SDE_TC_MASK_ICP			(SDE_TC_HOTPLUG_ICP(HPD_PORT_TC4) | \
>> >-					 SDE_TC_HOTPLUG_ICP(HPD_PORT_TC3) | \
>> >-					 SDE_TC_HOTPLUG_ICP(HPD_PORT_TC2) | \
>> >-					 SDE_TC_HOTPLUG_ICP(HPD_PORT_TC1))
>> >-#define SDE_DDI_MASK_TGP		(SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \
>> >+#define SDE_DDI_MASK_ICP		(SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \
>> > 					 SDE_DDI_HOTPLUG_ICP(HPD_PORT_B) | \
>> > 					 SDE_DDI_HOTPLUG_ICP(HPD_PORT_A))
>>
>> and here is the opportunity to rename this to SDE_DDI_HOTPLUG_MASK_ICP
>> or in a separate patch.
>>
>> with the patch reorder,
>>
>>
>> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>> Lucas De Marchi
>>
>> >-#define SDE_TC_MASK_TGP			(SDE_TC_HOTPLUG_ICP(HPD_PORT_TC6) | \
>> >+#define SDE_TC_MASK_ICP			(SDE_TC_HOTPLUG_ICP(HPD_PORT_TC6) | \
>> > 					 SDE_TC_HOTPLUG_ICP(HPD_PORT_TC5) | \
>> > 					 SDE_TC_HOTPLUG_ICP(HPD_PORT_TC4) | \
>> > 					 SDE_TC_HOTPLUG_ICP(HPD_PORT_TC3) | \
>> >--
>> >2.26.2
>> >
>> >_______________________________________________
>> >Intel-gfx mailing list
>> >Intel-gfx@lists.freedesktop.org
>> >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>-- 
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx