diff mbox series

[v2,03/14] drm/i915/bios: reduce indent in sanitize_ddc_pin and sanitize_aux_ch

Message ID 17288137452f731a820e737582672f836660a26f.1615998927.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/bios: vbt child device rework | expand

Commit Message

Jani Nikula March 17, 2021, 4:36 p.m. UTC
Reduce indent with an early return. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 86 +++++++++++------------
 1 file changed, 41 insertions(+), 45 deletions(-)

Comments

Lucas De Marchi March 17, 2021, 7:18 p.m. UTC | #1
On Wed, Mar 17, 2021 at 06:36:42PM +0200, Jani Nikula wrote:
>Reduce indent with an early return. No functional changes.
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_bios.c | 86 +++++++++++------------
> 1 file changed, 41 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>index 824148063451..5e7dc0899ab1 100644
>--- a/drivers/gpu/drm/i915/display/intel_bios.c
>+++ b/drivers/gpu/drm/i915/display/intel_bios.c
>@@ -1525,31 +1525,29 @@ static void sanitize_ddc_pin(struct drm_i915_private *i915,
> 		return;
>
> 	p = get_port_by_ddc_pin(i915, info->alternate_ddc_pin);
>-	if (p != PORT_NONE) {
>-		drm_dbg_kms(&i915->drm,
>-			    "port %c trying to use the same DDC pin (0x%x) as port %c, "
>-			    "disabling port %c DVI/HDMI support\n",
>-			    port_name(port), info->alternate_ddc_pin,
>-			    port_name(p), port_name(p));
>+	if (p == PORT_NONE)
>+		return;
>
>-		/*
>-		 * If we have multiple ports supposedly sharing the
>-		 * pin, then dvi/hdmi couldn't exist on the shared
>-		 * port. Otherwise they share the same ddc bin and
>-		 * system couldn't communicate with them separately.
>-		 *
>-		 * Give inverse child device order the priority,
>-		 * last one wins. Yes, there are real machines
>-		 * (eg. Asrock B250M-HDV) where VBT has both
>-		 * port A and port E with the same AUX ch and
>-		 * we must pick port E :(
>-		 */
>-		info = &i915->vbt.ddi_port_info[p];
>+	drm_dbg_kms(&i915->drm,
>+		    "port %c trying to use the same DDC pin (0x%x) as port %c, "
>+		    "disabling port %c DVI/HDMI support\n",
>+		    port_name(port), info->alternate_ddc_pin,
>+		    port_name(p), port_name(p));
>
>-		info->supports_dvi = false;
>-		info->supports_hdmi = false;
>-		info->alternate_ddc_pin = 0;
>-	}
>+	/*
>+	 * If we have multiple ports supposedly sharing the pin, then dvi/hdmi
>+	 * couldn't exist on the shared port. Otherwise they share the same ddc
>+	 * bin and system couldn't communicate with them separately.

while at it, may be worth fixing the typo s/bin/pin/


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

>+	 *
>+	 * Give inverse child device order the priority, last one wins. Yes,
>+	 * there are real machines (eg. Asrock B250M-HDV) where VBT has both
>+	 * port A and port E with the same AUX ch and we must pick port E :(
>+	 */
>+	info = &i915->vbt.ddi_port_info[p];
>+
>+	info->supports_dvi = false;
>+	info->supports_hdmi = false;
>+	info->alternate_ddc_pin = 0;
> }
>
> static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
>@@ -1577,30 +1575,28 @@ static void sanitize_aux_ch(struct drm_i915_private *i915,
> 		return;
>
> 	p = get_port_by_aux_ch(i915, info->alternate_aux_channel);
>-	if (p != PORT_NONE) {
>-		drm_dbg_kms(&i915->drm,
>-			    "port %c trying to use the same AUX CH (0x%x) as port %c, "
>-			    "disabling port %c DP support\n",
>-			    port_name(port), info->alternate_aux_channel,
>-			    port_name(p), port_name(p));
>+	if (p == PORT_NONE)
>+		return;
>
>-		/*
>-		 * If we have multiple ports supposedlt sharing the
>-		 * aux channel, then DP couldn't exist on the shared
>-		 * port. Otherwise they share the same aux channel
>-		 * and system couldn't communicate with them separately.
>-		 *
>-		 * Give inverse child device order the priority,
>-		 * last one wins. Yes, there are real machines
>-		 * (eg. Asrock B250M-HDV) where VBT has both
>-		 * port A and port E with the same AUX ch and
>-		 * we must pick port E :(
>-		 */
>-		info = &i915->vbt.ddi_port_info[p];
>+	drm_dbg_kms(&i915->drm,
>+		    "port %c trying to use the same AUX CH (0x%x) as port %c, "
>+		    "disabling port %c DP support\n",
>+		    port_name(port), info->alternate_aux_channel,
>+		    port_name(p), port_name(p));
>
>-		info->supports_dp = false;
>-		info->alternate_aux_channel = 0;
>-	}
>+	/*
>+	 * If we have multiple ports supposedlt sharing the aux channel, then DP
>+	 * couldn't exist on the shared port. Otherwise they share the same aux
>+	 * channel and system couldn't communicate with them separately.
>+	 *
>+	 * Give inverse child device order the priority, last one wins. Yes,
>+	 * there are real machines (eg. Asrock B250M-HDV) where VBT has both
>+	 * port A and port E with the same AUX ch and we must pick port E :(
>+	 */
>+	info = &i915->vbt.ddi_port_info[p];
>+
>+	info->supports_dp = false;
>+	info->alternate_aux_channel = 0;
> }
>
> static const u8 cnp_ddc_pin_map[] = {
>-- 
>2.20.1
>
Lucas De Marchi March 17, 2021, 7:23 p.m. UTC | #2
On Wed, Mar 17, 2021 at 06:36:42PM +0200, Jani Nikula wrote:
>Reduce indent with an early return. No functional changes.
>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_bios.c | 86 +++++++++++------------
> 1 file changed, 41 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>index 824148063451..5e7dc0899ab1 100644
>--- a/drivers/gpu/drm/i915/display/intel_bios.c
>+++ b/drivers/gpu/drm/i915/display/intel_bios.c
>@@ -1525,31 +1525,29 @@ static void sanitize_ddc_pin(struct drm_i915_private *i915,
> 		return;
>
> 	p = get_port_by_ddc_pin(i915, info->alternate_ddc_pin);
>-	if (p != PORT_NONE) {
>-		drm_dbg_kms(&i915->drm,
>-			    "port %c trying to use the same DDC pin (0x%x) as port %c, "
>-			    "disabling port %c DVI/HDMI support\n",
>-			    port_name(port), info->alternate_ddc_pin,
>-			    port_name(p), port_name(p));
>+	if (p == PORT_NONE)
>+		return;
>
>-		/*
>-		 * If we have multiple ports supposedly sharing the
>-		 * pin, then dvi/hdmi couldn't exist on the shared
>-		 * port. Otherwise they share the same ddc bin and
>-		 * system couldn't communicate with them separately.
>-		 *
>-		 * Give inverse child device order the priority,
>-		 * last one wins. Yes, there are real machines
>-		 * (eg. Asrock B250M-HDV) where VBT has both
>-		 * port A and port E with the same AUX ch and
>-		 * we must pick port E :(
>-		 */
>-		info = &i915->vbt.ddi_port_info[p];
>+	drm_dbg_kms(&i915->drm,
>+		    "port %c trying to use the same DDC pin (0x%x) as port %c, "
>+		    "disabling port %c DVI/HDMI support\n",
>+		    port_name(port), info->alternate_ddc_pin,
>+		    port_name(p), port_name(p));
>
>-		info->supports_dvi = false;
>-		info->supports_hdmi = false;
>-		info->alternate_ddc_pin = 0;
>-	}
>+	/*
>+	 * If we have multiple ports supposedly sharing the pin, then dvi/hdmi
>+	 * couldn't exist on the shared port. Otherwise they share the same ddc
>+	 * bin and system couldn't communicate with them separately.
>+	 *
>+	 * Give inverse child device order the priority, last one wins. Yes,
>+	 * there are real machines (eg. Asrock B250M-HDV) where VBT has both
>+	 * port A and port E with the same AUX ch and we must pick port E :(
>+	 */
>+	info = &i915->vbt.ddi_port_info[p];
>+
>+	info->supports_dvi = false;
>+	info->supports_hdmi = false;
>+	info->alternate_ddc_pin = 0;
> }
>
> static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
>@@ -1577,30 +1575,28 @@ static void sanitize_aux_ch(struct drm_i915_private *i915,
> 		return;
>
> 	p = get_port_by_aux_ch(i915, info->alternate_aux_channel);
>-	if (p != PORT_NONE) {
>-		drm_dbg_kms(&i915->drm,
>-			    "port %c trying to use the same AUX CH (0x%x) as port %c, "
>-			    "disabling port %c DP support\n",
>-			    port_name(port), info->alternate_aux_channel,
>-			    port_name(p), port_name(p));
>+	if (p == PORT_NONE)
>+		return;
>
>-		/*
>-		 * If we have multiple ports supposedlt sharing the
>-		 * aux channel, then DP couldn't exist on the shared
>-		 * port. Otherwise they share the same aux channel
>-		 * and system couldn't communicate with them separately.
>-		 *
>-		 * Give inverse child device order the priority,
>-		 * last one wins. Yes, there are real machines
>-		 * (eg. Asrock B250M-HDV) where VBT has both
>-		 * port A and port E with the same AUX ch and
>-		 * we must pick port E :(
>-		 */
>-		info = &i915->vbt.ddi_port_info[p];
>+	drm_dbg_kms(&i915->drm,
>+		    "port %c trying to use the same AUX CH (0x%x) as port %c, "
>+		    "disabling port %c DP support\n",
>+		    port_name(port), info->alternate_aux_channel,
>+		    port_name(p), port_name(p));
>
>-		info->supports_dp = false;
>-		info->alternate_aux_channel = 0;
>-	}
>+	/*
>+	 * If we have multiple ports supposedlt sharing the aux channel, then DP

and another typo: supposedly. Might as well be a patch on top fixing
these, up to you.

Lucas De Marchi

>+	 * couldn't exist on the shared port. Otherwise they share the same aux
>+	 * channel and system couldn't communicate with them separately.
>+	 *
>+	 * Give inverse child device order the priority, last one wins. Yes,
>+	 * there are real machines (eg. Asrock B250M-HDV) where VBT has both
>+	 * port A and port E with the same AUX ch and we must pick port E :(
>+	 */
>+	info = &i915->vbt.ddi_port_info[p];
>+
>+	info->supports_dp = false;
>+	info->alternate_aux_channel = 0;
> }
>
> static const u8 cnp_ddc_pin_map[] = {
>-- 
>2.20.1
>
Jani Nikula March 18, 2021, 12:47 p.m. UTC | #3
On Wed, 17 Mar 2021, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Wed, Mar 17, 2021 at 06:36:42PM +0200, Jani Nikula wrote:
>>Reduce indent with an early return. No functional changes.
>>
>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>---
>> drivers/gpu/drm/i915/display/intel_bios.c | 86 +++++++++++------------
>> 1 file changed, 41 insertions(+), 45 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>>index 824148063451..5e7dc0899ab1 100644
>>--- a/drivers/gpu/drm/i915/display/intel_bios.c
>>+++ b/drivers/gpu/drm/i915/display/intel_bios.c
>>@@ -1525,31 +1525,29 @@ static void sanitize_ddc_pin(struct drm_i915_private *i915,
>> 		return;
>>
>> 	p = get_port_by_ddc_pin(i915, info->alternate_ddc_pin);
>>-	if (p != PORT_NONE) {
>>-		drm_dbg_kms(&i915->drm,
>>-			    "port %c trying to use the same DDC pin (0x%x) as port %c, "
>>-			    "disabling port %c DVI/HDMI support\n",
>>-			    port_name(port), info->alternate_ddc_pin,
>>-			    port_name(p), port_name(p));
>>+	if (p == PORT_NONE)
>>+		return;
>>
>>-		/*
>>-		 * If we have multiple ports supposedly sharing the
>>-		 * pin, then dvi/hdmi couldn't exist on the shared
>>-		 * port. Otherwise they share the same ddc bin and
>>-		 * system couldn't communicate with them separately.
>>-		 *
>>-		 * Give inverse child device order the priority,
>>-		 * last one wins. Yes, there are real machines
>>-		 * (eg. Asrock B250M-HDV) where VBT has both
>>-		 * port A and port E with the same AUX ch and
>>-		 * we must pick port E :(
>>-		 */
>>-		info = &i915->vbt.ddi_port_info[p];
>>+	drm_dbg_kms(&i915->drm,
>>+		    "port %c trying to use the same DDC pin (0x%x) as port %c, "
>>+		    "disabling port %c DVI/HDMI support\n",
>>+		    port_name(port), info->alternate_ddc_pin,
>>+		    port_name(p), port_name(p));
>>
>>-		info->supports_dvi = false;
>>-		info->supports_hdmi = false;
>>-		info->alternate_ddc_pin = 0;
>>-	}
>>+	/*
>>+	 * If we have multiple ports supposedly sharing the pin, then dvi/hdmi
>>+	 * couldn't exist on the shared port. Otherwise they share the same ddc
>>+	 * bin and system couldn't communicate with them separately.
>>+	 *
>>+	 * Give inverse child device order the priority, last one wins. Yes,
>>+	 * there are real machines (eg. Asrock B250M-HDV) where VBT has both
>>+	 * port A and port E with the same AUX ch and we must pick port E :(
>>+	 */
>>+	info = &i915->vbt.ddi_port_info[p];
>>+
>>+	info->supports_dvi = false;
>>+	info->supports_hdmi = false;
>>+	info->alternate_ddc_pin = 0;
>> }
>>
>> static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
>>@@ -1577,30 +1575,28 @@ static void sanitize_aux_ch(struct drm_i915_private *i915,
>> 		return;
>>
>> 	p = get_port_by_aux_ch(i915, info->alternate_aux_channel);
>>-	if (p != PORT_NONE) {
>>-		drm_dbg_kms(&i915->drm,
>>-			    "port %c trying to use the same AUX CH (0x%x) as port %c, "
>>-			    "disabling port %c DP support\n",
>>-			    port_name(port), info->alternate_aux_channel,
>>-			    port_name(p), port_name(p));
>>+	if (p == PORT_NONE)
>>+		return;
>>
>>-		/*
>>-		 * If we have multiple ports supposedlt sharing the
>>-		 * aux channel, then DP couldn't exist on the shared
>>-		 * port. Otherwise they share the same aux channel
>>-		 * and system couldn't communicate with them separately.
>>-		 *
>>-		 * Give inverse child device order the priority,
>>-		 * last one wins. Yes, there are real machines
>>-		 * (eg. Asrock B250M-HDV) where VBT has both
>>-		 * port A and port E with the same AUX ch and
>>-		 * we must pick port E :(
>>-		 */
>>-		info = &i915->vbt.ddi_port_info[p];
>>+	drm_dbg_kms(&i915->drm,
>>+		    "port %c trying to use the same AUX CH (0x%x) as port %c, "
>>+		    "disabling port %c DP support\n",
>>+		    port_name(port), info->alternate_aux_channel,
>>+		    port_name(p), port_name(p));
>>
>>-		info->supports_dp = false;
>>-		info->alternate_aux_channel = 0;
>>-	}
>>+	/*
>>+	 * If we have multiple ports supposedlt sharing the aux channel, then DP
>
> and another typo: supposedly. Might as well be a patch on top fixing
> these, up to you.

As they're just comments, I took the liberty of fixing while pushing.

Pushed the entire series, thanks for the review.

BR,
Jani.


>
> Lucas De Marchi
>
>>+	 * couldn't exist on the shared port. Otherwise they share the same aux
>>+	 * channel and system couldn't communicate with them separately.
>>+	 *
>>+	 * Give inverse child device order the priority, last one wins. Yes,
>>+	 * there are real machines (eg. Asrock B250M-HDV) where VBT has both
>>+	 * port A and port E with the same AUX ch and we must pick port E :(
>>+	 */
>>+	info = &i915->vbt.ddi_port_info[p];
>>+
>>+	info->supports_dp = false;
>>+	info->alternate_aux_channel = 0;
>> }
>>
>> static const u8 cnp_ddc_pin_map[] = {
>>-- 
>>2.20.1
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 824148063451..5e7dc0899ab1 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1525,31 +1525,29 @@  static void sanitize_ddc_pin(struct drm_i915_private *i915,
 		return;
 
 	p = get_port_by_ddc_pin(i915, info->alternate_ddc_pin);
-	if (p != PORT_NONE) {
-		drm_dbg_kms(&i915->drm,
-			    "port %c trying to use the same DDC pin (0x%x) as port %c, "
-			    "disabling port %c DVI/HDMI support\n",
-			    port_name(port), info->alternate_ddc_pin,
-			    port_name(p), port_name(p));
+	if (p == PORT_NONE)
+		return;
 
-		/*
-		 * If we have multiple ports supposedly sharing the
-		 * pin, then dvi/hdmi couldn't exist on the shared
-		 * port. Otherwise they share the same ddc bin and
-		 * system couldn't communicate with them separately.
-		 *
-		 * Give inverse child device order the priority,
-		 * last one wins. Yes, there are real machines
-		 * (eg. Asrock B250M-HDV) where VBT has both
-		 * port A and port E with the same AUX ch and
-		 * we must pick port E :(
-		 */
-		info = &i915->vbt.ddi_port_info[p];
+	drm_dbg_kms(&i915->drm,
+		    "port %c trying to use the same DDC pin (0x%x) as port %c, "
+		    "disabling port %c DVI/HDMI support\n",
+		    port_name(port), info->alternate_ddc_pin,
+		    port_name(p), port_name(p));
 
-		info->supports_dvi = false;
-		info->supports_hdmi = false;
-		info->alternate_ddc_pin = 0;
-	}
+	/*
+	 * If we have multiple ports supposedly sharing the pin, then dvi/hdmi
+	 * couldn't exist on the shared port. Otherwise they share the same ddc
+	 * bin and system couldn't communicate with them separately.
+	 *
+	 * Give inverse child device order the priority, last one wins. Yes,
+	 * there are real machines (eg. Asrock B250M-HDV) where VBT has both
+	 * port A and port E with the same AUX ch and we must pick port E :(
+	 */
+	info = &i915->vbt.ddi_port_info[p];
+
+	info->supports_dvi = false;
+	info->supports_hdmi = false;
+	info->alternate_ddc_pin = 0;
 }
 
 static enum port get_port_by_aux_ch(struct drm_i915_private *i915, u8 aux_ch)
@@ -1577,30 +1575,28 @@  static void sanitize_aux_ch(struct drm_i915_private *i915,
 		return;
 
 	p = get_port_by_aux_ch(i915, info->alternate_aux_channel);
-	if (p != PORT_NONE) {
-		drm_dbg_kms(&i915->drm,
-			    "port %c trying to use the same AUX CH (0x%x) as port %c, "
-			    "disabling port %c DP support\n",
-			    port_name(port), info->alternate_aux_channel,
-			    port_name(p), port_name(p));
+	if (p == PORT_NONE)
+		return;
 
-		/*
-		 * If we have multiple ports supposedlt sharing the
-		 * aux channel, then DP couldn't exist on the shared
-		 * port. Otherwise they share the same aux channel
-		 * and system couldn't communicate with them separately.
-		 *
-		 * Give inverse child device order the priority,
-		 * last one wins. Yes, there are real machines
-		 * (eg. Asrock B250M-HDV) where VBT has both
-		 * port A and port E with the same AUX ch and
-		 * we must pick port E :(
-		 */
-		info = &i915->vbt.ddi_port_info[p];
+	drm_dbg_kms(&i915->drm,
+		    "port %c trying to use the same AUX CH (0x%x) as port %c, "
+		    "disabling port %c DP support\n",
+		    port_name(port), info->alternate_aux_channel,
+		    port_name(p), port_name(p));
 
-		info->supports_dp = false;
-		info->alternate_aux_channel = 0;
-	}
+	/*
+	 * If we have multiple ports supposedlt sharing the aux channel, then DP
+	 * couldn't exist on the shared port. Otherwise they share the same aux
+	 * channel and system couldn't communicate with them separately.
+	 *
+	 * Give inverse child device order the priority, last one wins. Yes,
+	 * there are real machines (eg. Asrock B250M-HDV) where VBT has both
+	 * port A and port E with the same AUX ch and we must pick port E :(
+	 */
+	info = &i915->vbt.ddi_port_info[p];
+
+	info->supports_dp = false;
+	info->alternate_aux_channel = 0;
 }
 
 static const u8 cnp_ddc_pin_map[] = {