On Tue, Feb 28, 2023 at 08:36:16PM +0200, Jani Nikula wrote:
> On Wed, 22 Feb 2023, Ville Syrjala <[email protected]> wrote:
> > From: Ville Syrjälä <[email protected]>
> >
> > Pair each <platform>_hotplug_enables() function with
> > a corresponding <platform>_hotplug_mask() function so that
> > we can determine right bits to clear on a per hpd_pin basis.
> > We'll need this for turning on HPD sense for a specific
> > encoder rather than just all of them.
> >
> > Signed-off-by: Ville Syrjälä <[email protected]>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 231 ++++++++++++++++++++++----------
> > 1 file changed, 160 insertions(+), 71 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 13ada0916c2a..ecfa6dad145a 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2835,8 +2835,25 @@ static void cherryview_irq_reset(struct
> > drm_i915_private *dev_priv)
> > vlv_display_irq_reset(dev_priv);
> > spin_unlock_irq(&dev_priv->irq_lock);
> > }
> >
> > +static u32 ibx_hotplug_mask(struct drm_i915_private *i915,
> > + enum hpd_pin hpd_pin)
>
>
> What's the reason for passing i915 to these functions? I see no use in
> this series, am I missing something? Seems like it makes some calls a
> bit convoluted.
>
> BR,
> Jani.
>
> > +{
> > + switch (hpd_pin) {
> > + case HPD_PORT_A:
I think I initially copied over the LPT-LP check from
ibx_hotplug_enables(), which is what needed i915. But
apparently I got rid of it later, and so looks like
i915 can go too.
> > + return PORTA_HOTPLUG_ENABLE;
> > + case HPD_PORT_B:
> > + return PORTB_HOTPLUG_ENABLE | PORTB_PULSE_DURATION_MASK;
> > + case HPD_PORT_C:
> > + return PORTC_HOTPLUG_ENABLE | PORTC_PULSE_DURATION_MASK;
> > + case HPD_PORT_D:
> > + return PORTD_HOTPLUG_ENABLE | PORTD_PULSE_DURATION_MASK;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > static u32 ibx_hotplug_enables(struct intel_encoder *encoder)
> > {
> > struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >
> > @@ -2869,15 +2886,12 @@ static void ibx_hpd_detection_setup(struct
> > drm_i915_private *dev_priv)
> > * duration to 2ms (which is the minimum in the Display Port spec).
> > * The pulse duration bits are reserved on LPT+.
> > */
> > intel_uncore_rmw(&dev_priv->uncore, PCH_PORT_HOTPLUG,
> > - PORTA_HOTPLUG_ENABLE |
No LPT-LP check here either, so presumably clearing the bit
unconditionally is fine... Yeah, doesn't look like there was
anything else there, ever.
> > - PORTB_HOTPLUG_ENABLE |
> > - PORTC_HOTPLUG_ENABLE |
> > - PORTD_HOTPLUG_ENABLE |
> > - PORTB_PULSE_DURATION_MASK |
> > - PORTC_PULSE_DURATION_MASK |
> > - PORTD_PULSE_DURATION_MASK,
> > + ibx_hotplug_mask(dev_priv, HPD_PORT_A) |
> > + ibx_hotplug_mask(dev_priv, HPD_PORT_B) |
> > + ibx_hotplug_mask(dev_priv, HPD_PORT_C) |
> > + ibx_hotplug_mask(dev_priv, HPD_PORT_D),
> > intel_hpd_hotplug_enables(dev_priv,
> > ibx_hotplug_enables));
> > }
> >
--
Ville Syrjälä
Intel
