From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sivakumar Thulasimani Subject: Re: [PATCH v2 2/3] drm/i915: don't use HPD_PORT_A as an alias for HPD_NONE Date: Wed, 22 Jul 2015 13:58:20 +0530 Message-ID: <55AF5424.1010800@intel.com> References: <1437428619-30160-3-git-send-email-imre.deak@intel.com> <1437517965-2605-2-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2005039310==" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 1E9736E509 for ; Wed, 22 Jul 2015 01:28:25 -0700 (PDT) In-Reply-To: <1437517965-2605-2-git-send-email-imre.deak@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Imre Deak , intel-gfx@lists.freedesktop.org Cc: Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============2005039310== Content-Type: multipart/alternative; boundary="------------010901010406030105090002" This is a multi-part message in MIME format. --------------010901010406030105090002 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit On 7/22/2015 4:02 AM, Imre Deak wrote: > Curren tly 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. > > v2: > - rebase on top of -nightly (Daniel) > - make the check for intel_hpd_pin_to_port() return value more readable > (Sivakumar) > > Signed-off-by: Imre Deak > Reviewed-by: Sonika Jindal > Reviewed-by: Sivakumar Thulasimani > --- > 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, 18 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index b876e78..b94ada9 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, > @@ -2648,7 +2648,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 9410aab..f08ec9c 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1273,7 +1273,9 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask, > > *pin_mask |= BIT(i); > > - port = intel_hpd_pin_to_port(i); > + if (!intel_hpd_pin_to_port(i, &port)) > + continue; > + > if (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 3c9171f..032a0bf 100644 > --- a/drivers/gpu/drm/i915/intel_hotplug.c > +++ b/drivers/gpu/drm/i915/intel_hotplug.c > @@ -76,17 +76,23 @@ > * it will use i915_hotplug_work_func where this logic is handled. > */ > > -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 */ > } > } > > @@ -369,8 +375,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); Reviewed-by: Sivakumar Thulasimani -- regards, Sivakumar --------------010901010406030105090002 Content-Type: text/html; charset=windows-1252 Content-Transfer-Encoding: 7bit

On 7/22/2015 4:02 AM, Imre Deak wrote:
Curren tly 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.

v2:
- rebase on top of -nightly (Daniel)
- make the check for intel_hpd_pin_to_port() return value more readable
  (Sivakumar)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@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, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b876e78..b94ada9 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,
@@ -2648,7 +2648,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 9410aab..f08ec9c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1273,7 +1273,9 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
 
 		*pin_mask |= BIT(i);
 
-		port = intel_hpd_pin_to_port(i);
+		if (!intel_hpd_pin_to_port(i, &port))
+			continue;
+
 		if (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 3c9171f..032a0bf 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -76,17 +76,23 @@
  * it will use i915_hotplug_work_func where this logic is handled.
  */
 
-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 */
 	}
 }
 
@@ -369,8 +375,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);

Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

-- 
regards,
Sivakumar
--------------010901010406030105090002-- --===============2005039310== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============2005039310==--