[Intel-gfx] [PATCH 2/4] drm/i915: unify icp, tgp and mcc irq handling

[Souza, Jose]

On Thu, 2019-08-29 at 16:05 -0700, Matt Roper wrote:
> On Thu, Aug 29, 2019 at 02:15:24PM -0700, José Roberto de Souza
> wrote:
> > From: Lucas De Marchi <lucas.demarchi at intel.com>
> > 
> > The differences are only on the pins, trigger and long_detect
> > function.
> > The MCC handling is already partially merged, so merge TGP as well.
> > Remove the pins argument from icp_irq_handler() so we have all the
> > differences between the 3 set in a common if ladder.
> > 
> > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> 
> Now that everything is parameterized would it be worth unifying the
> tc
> long detect functions too?  E.g., something like
> 
>     if (HAS_PCH_TGP(dev_priv))
>         return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1 + pin -
> HPD_PORT_D);
>     else if (HAS_PCH_ICP(dev_priv))
>         return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1 + pin -
> HPD_PORT_C);
>     else
>         MISSING_CASE(INTEL_PCH_TYPE(dev_priv));
> 
> Even if you decide to keep it as is, this patch is
> 
> Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


We can do it on top but I personally don't like keep this much detail
of register in .c files.

Thanks for the reviews.

> 
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 65 ++++++++---------------------
> > ----
> >  1 file changed, 16 insertions(+), 49 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 084e322ec15b..5f590987dcd5 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2243,19 +2243,27 @@ static void cpt_irq_handler(struct
> > drm_i915_private *dev_priv, u32 pch_iir)
> >  		cpt_serr_int_handler(dev_priv);
> >  }
> >  
> > -static void icp_irq_handler(struct drm_i915_private *dev_priv, u32
> > pch_iir,
> > -			    const u32 *pins)
> > +static void icp_irq_handler(struct drm_i915_private *dev_priv, u32
> > pch_iir)
> >  {
> > -	u32 ddi_hotplug_trigger;
> > -	u32 tc_hotplug_trigger;
> > +	u32 ddi_hotplug_trigger, tc_hotplug_trigger;
> >  	u32 pin_mask = 0, long_mask = 0;
> > +	bool (*tc_port_hotplug_long_detect)(enum hpd_pin pin, u32 val);
> > +	const u32 *pins;
> >  
> > -	if (HAS_PCH_MCC(dev_priv)) {
> > +	if (HAS_PCH_TGP(dev_priv)) {
> > +		ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_TGP;
> > +		tc_hotplug_trigger = pch_iir & SDE_TC_MASK_TGP;
> > +		tc_port_hotplug_long_detect =
> > tgp_tc_port_hotplug_long_detect;
> > +		pins = hpd_tgp;
> > +	} else if (HAS_PCH_MCC(dev_priv)) {
> >  		ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_TGP;
> >  		tc_hotplug_trigger = 0;
> > +		pins = hpd_mcc;
> >  	} else {
> >  		ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
> >  		tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
> > +		tc_port_hotplug_long_detect =
> > icp_tc_port_hotplug_long_detect;
> > +		pins = hpd_icp;
> >  	}
> >  
> >  	if (ddi_hotplug_trigger) {
> > @@ -2279,44 +2287,7 @@ static void icp_irq_handler(struct
> > drm_i915_private *dev_priv, u32 pch_iir,
> >  		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> >  				   tc_hotplug_trigger,
> >  				   dig_hotplug_reg, pins,
> > -				   icp_tc_port_hotplug_long_detect);
> > -	}
> > -
> > -	if (pin_mask)
> > -		intel_hpd_irq_handler(dev_priv, pin_mask, long_mask);
> > -
> > -	if (pch_iir & SDE_GMBUS_ICP)
> > -		gmbus_irq_handler(dev_priv);
> > -}
> > -
> > -static void tgp_irq_handler(struct drm_i915_private *dev_priv, u32
> > pch_iir)
> > -{
> > -	u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_TGP;
> > -	u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_TGP;
> > -	u32 pin_mask = 0, long_mask = 0;
> > -
> > -	if (ddi_hotplug_trigger) {
> > -		u32 dig_hotplug_reg;
> > -
> > -		dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI);
> > -		I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg);
> > -
> > -		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> > -				   ddi_hotplug_trigger,
> > -				   dig_hotplug_reg, hpd_tgp,
> > -				   icp_ddi_port_hotplug_long_detect);
> > -	}
> > -
> > -	if (tc_hotplug_trigger) {
> > -		u32 dig_hotplug_reg;
> > -
> > -		dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC);
> > -		I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg);
> > -
> > -		intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> > -				   tc_hotplug_trigger,
> > -				   dig_hotplug_reg, hpd_tgp,
> > -				   tgp_tc_port_hotplug_long_detect);
> > +				   tc_port_hotplug_long_detect);
> >  	}
> >  
> >  	if (pin_mask)
> > @@ -2767,12 +2738,8 @@ gen8_de_irq_handler(struct drm_i915_private
> > *dev_priv, u32 master_ctl)
> >  			I915_WRITE(SDEIIR, iir);
> >  			ret = IRQ_HANDLED;
> >  
> > -			if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP)
> > -				tgp_irq_handler(dev_priv, iir);
> > -			else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MCC)
> > -				icp_irq_handler(dev_priv, iir,
> > hpd_mcc);
> > -			else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> > -				icp_irq_handler(dev_priv, iir,
> > hpd_icp);
> > +			if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> > +				icp_irq_handler(dev_priv, iir);
> >  			else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
> >  				spt_irq_handler(dev_priv, iir);
> >  			else
> > -- 
> > 2.23.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx