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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
>
> 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@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
