On Tue, 2015-07-21 at 13:51 +0530, Sivakumar Thulasimani wrote:
>
>
> On 7/21/2015 3:13 AM, Imre Deak wrote:
>
> > Currently HPD_PORT_A is used as an alias for HPD_NONE to mean that the
> > given port doesn't support long/short HPD pulse detection. SDVO and CRT
> > ports are like this and for these ports we only want to know whether an
> > hot plug event was detected on the corresponding pin. Since at least on
> > BXT we need long/short pulse detection on PORT A as well (added by the
> > next patch) remove this aliasing of HPD_PORT_A/HPD_NONE and let the
> > return value of intel_hpd_pin_to_port() show whether long/short pulse
> > detection is supported on the passed in pin.
> >
> > No functional change.
> >
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 4 ++--
> > drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> > drivers/gpu/drm/i915/intel_hotplug.c | 20 +++++++++++++-------
> > 3 files changed, 17 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 78d6c7a..ebfd5a5 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -206,11 +206,11 @@ enum intel_display_power_domain {
> >
> > enum hpd_pin {
> > HPD_NONE = 0,
> > - HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
> > HPD_TV = HPD_NONE, /* TV is known to be unreliable */
> > HPD_CRT,
> > HPD_SDVO_B,
> > HPD_SDVO_C,
> > + HPD_PORT_A,
> > HPD_PORT_B,
> > HPD_PORT_C,
> > HPD_PORT_D,
> > @@ -2627,7 +2627,7 @@ void intel_hpd_irq_handler(struct drm_device *dev, u32 pin_mask, u32 long_mask);
> > void intel_hpd_init(struct drm_i915_private *dev_priv);
> > void intel_hpd_init_work(struct drm_i915_private *dev_priv);
> > void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
> > -enum port intel_hpd_pin_to_port(enum hpd_pin pin);
> > +bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port);
> >
> > /* i915_irq.c */
> > void i915_queue_hangcheck(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 306de78..4ad7a31 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1276,8 +1276,8 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> >
> > *pin_mask |= BIT(i);
> >
> > - port = intel_hpd_pin_to_port(i);
> > - if (long_pulse_detect(port, dig_hotplug_reg))
> > + if (intel_hpd_pin_to_port(i, &port) &&
> > + long_pulse_detect(port, dig_hotplug_reg))
> > *long_mask |= BIT(i);
> > }
> feel that this could be more readable when split it to two
> conditions ?
> if (intel_hpd_pin_to_port(i, &port) == false)
> continue;
> if (long_pulse_detect(port, dig_hotplug_reg)
> *long_mask |= BIT(i);
Ok, will change it.
>
> other than this, this change looks good.
>
>
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@xxxxxxxxx>
>
>
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 3c53aac..8cda7b9 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -29,17 +29,23 @@
> > #include "i915_drv.h"
> > #include "intel_drv.h"
> >
> > -enum port intel_hpd_pin_to_port(enum hpd_pin pin)
> > +bool intel_hpd_pin_to_port(enum hpd_pin pin, enum port *port)
> > {
> > switch (pin) {
> > + case HPD_PORT_A:
> > + *port = PORT_A;
> > + return true;
> > case HPD_PORT_B:
> > - return PORT_B;
> > + *port = PORT_B;
> > + return true;
> > case HPD_PORT_C:
> > - return PORT_C;
> > + *port = PORT_C;
> > + return true;
> > case HPD_PORT_D:
> > - return PORT_D;
> > + *port = PORT_D;
> > + return true;
> > default:
> > - return PORT_A; /* no hpd */
> > + return false; /* no hpd */
> > }
> > }
> >
> > @@ -322,8 +328,8 @@ void intel_hpd_irq_handler(struct drm_device *dev,
> > if (!(BIT(i) & pin_mask))
> > continue;
> >
> > - port = intel_hpd_pin_to_port(i);
> > - is_dig_port = port && dev_priv->hotplug.irq_port[port];
> > + is_dig_port = intel_hpd_pin_to_port(i, &port) &&
> > + dev_priv->hotplug.irq_port[port];
> >
> > if (is_dig_port) {
> > bool long_hpd = long_mask & BIT(i);
>
> --
> regards,
> Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
