| Message ID | 1437428619-30160-3-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > 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); other than this, this change looks good. Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > > 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);
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@intel.com> > > --- > > 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@intel.com> > > > > > > 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
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); } 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);
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@intel.com> --- 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(-)