stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/8] drm/i915/sdvo: Filter out invalid outputs more sensibly
       [not found] <20221026101134.20865-1-ville.syrjala@linux.intel.com>
@ 2022-10-26 10:11 ` Ville Syrjala
  2022-10-27 14:32   ` [Intel-gfx] " Jani Nikula
  2022-10-26 10:11 ` [PATCH 2/8] drm/i915/sdvo: Setup DDC fully before output init Ville Syrjala
  2022-10-26 10:11 ` [PATCH 3/8] drm/i915/sdvo: Grab mode_config.mutex during LVDS init to avoid WARNs Ville Syrjala
  2 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjala @ 2022-10-26 10:11 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We try to filter out the corresponding xxx1 output
if the xxx0 output is not present. But the way that is
being done is pretty awkward. Make it less so.

Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_sdvo.c | 29 ++++++++++++++++++-----
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index cf8e80936d8e..c6200a91a777 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2925,16 +2925,33 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
 	return false;
 }
 
-static bool
-intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
+static u16 intel_sdvo_filter_output_flags(u16 flags)
 {
+	flags &= SDVO_OUTPUT_MASK;
+
 	/* SDVO requires XXX1 function may not exist unless it has XXX0 function.*/
+	if (!(flags & SDVO_OUTPUT_TMDS0))
+		flags &= ~SDVO_OUTPUT_TMDS1;
+
+	if (!(flags & SDVO_OUTPUT_RGB0))
+		flags &= ~SDVO_OUTPUT_RGB1;
+
+	if (!(flags & SDVO_OUTPUT_LVDS0))
+		flags &= ~SDVO_OUTPUT_LVDS1;
+
+	return flags;
+}
+
+static bool
+intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
+{
+	flags = intel_sdvo_filter_output_flags(flags);
 
 	if (flags & SDVO_OUTPUT_TMDS0)
 		if (!intel_sdvo_dvi_init(intel_sdvo, 0))
 			return false;
 
-	if ((flags & SDVO_TMDS_MASK) == SDVO_TMDS_MASK)
+	if (flags & SDVO_OUTPUT_TMDS1)
 		if (!intel_sdvo_dvi_init(intel_sdvo, 1))
 			return false;
 
@@ -2955,7 +2972,7 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
 		if (!intel_sdvo_analog_init(intel_sdvo, 0))
 			return false;
 
-	if ((flags & SDVO_RGB_MASK) == SDVO_RGB_MASK)
+	if (flags & SDVO_OUTPUT_RGB1)
 		if (!intel_sdvo_analog_init(intel_sdvo, 1))
 			return false;
 
@@ -2963,11 +2980,11 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
 		if (!intel_sdvo_lvds_init(intel_sdvo, 0))
 			return false;
 
-	if ((flags & SDVO_LVDS_MASK) == SDVO_LVDS_MASK)
+	if (flags & SDVO_OUTPUT_LVDS1)
 		if (!intel_sdvo_lvds_init(intel_sdvo, 1))
 			return false;
 
-	if ((flags & SDVO_OUTPUT_MASK) == 0) {
+	if (flags == 0) {
 		unsigned char bytes[2];
 
 		intel_sdvo->controlled_output = 0;
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/8] drm/i915/sdvo: Setup DDC fully before output init
       [not found] <20221026101134.20865-1-ville.syrjala@linux.intel.com>
  2022-10-26 10:11 ` [PATCH 1/8] drm/i915/sdvo: Filter out invalid outputs more sensibly Ville Syrjala
@ 2022-10-26 10:11 ` Ville Syrjala
  2022-10-27 14:36   ` [Intel-gfx] " Jani Nikula
  2022-10-26 10:11 ` [PATCH 3/8] drm/i915/sdvo: Grab mode_config.mutex during LVDS init to avoid WARNs Ville Syrjala
  2 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjala @ 2022-10-26 10:11 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Call intel_sdvo_select_ddc_bus() before initializing any
of the outputs. And before that is functional (assuming no VBT)
we have to set up the controlled_outputs thing. Otherwise DDC
won't be functional during the output init but LVDS really
needs it for the fixed mode setup.

Note that the whole multi output support still looks very
bogus, and more work will be needed to make it correct.
But for now this should at least fix the LVDS EDID fixed mode
setup.

Cc: stable@vger.kernel.org
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
Fixes: aa2b88074a56 ("drm/i915/sdvo: Fix multi function encoder stuff")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_sdvo.c | 31 +++++++++--------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index c6200a91a777..ccf81d616cb4 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2746,13 +2746,10 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
 	if (!intel_sdvo_connector)
 		return false;
 
-	if (device == 0) {
-		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0;
+	if (device == 0)
 		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS0;
-	} else if (device == 1) {
-		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1;
+	else if (device == 1)
 		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS1;
-	}
 
 	intel_connector = &intel_sdvo_connector->base;
 	connector = &intel_connector->base;
@@ -2807,7 +2804,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
 	encoder->encoder_type = DRM_MODE_ENCODER_TVDAC;
 	connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO;
 
-	intel_sdvo->controlled_output |= type;
 	intel_sdvo_connector->output_flag = type;
 
 	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
@@ -2848,13 +2844,10 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
 	encoder->encoder_type = DRM_MODE_ENCODER_DAC;
 	connector->connector_type = DRM_MODE_CONNECTOR_VGA;
 
-	if (device == 0) {
-		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0;
+	if (device == 0)
 		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB0;
-	} else if (device == 1) {
-		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1;
+	else if (device == 1)
 		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB1;
-	}
 
 	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
 		kfree(intel_sdvo_connector);
@@ -2884,13 +2877,10 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
 	encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
 	connector->connector_type = DRM_MODE_CONNECTOR_LVDS;
 
-	if (device == 0) {
-		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0;
+	if (device == 0)
 		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS0;
-	} else if (device == 1) {
-		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1;
+	else if (device == 1)
 		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS1;
-	}
 
 	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
 		kfree(intel_sdvo_connector);
@@ -2945,8 +2935,14 @@ static u16 intel_sdvo_filter_output_flags(u16 flags)
 static bool
 intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
 {
+	struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
+
 	flags = intel_sdvo_filter_output_flags(flags);
 
+	intel_sdvo->controlled_output = flags;
+
+	intel_sdvo_select_ddc_bus(i915, intel_sdvo);
+
 	if (flags & SDVO_OUTPUT_TMDS0)
 		if (!intel_sdvo_dvi_init(intel_sdvo, 0))
 			return false;
@@ -2987,7 +2983,6 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
 	if (flags == 0) {
 		unsigned char bytes[2];
 
-		intel_sdvo->controlled_output = 0;
 		memcpy(bytes, &intel_sdvo->caps.output_flags, 2);
 		DRM_DEBUG_KMS("%s: Unknown SDVO output type (0x%02x%02x)\n",
 			      SDVO_NAME(intel_sdvo),
@@ -3399,8 +3394,6 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
 	 */
 	intel_sdvo->base.cloneable = 0;
 
-	intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo);
-
 	/* Set the input timing to the screen. Assume always input 0. */
 	if (!intel_sdvo_set_target_input(intel_sdvo))
 		goto err_output;
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/8] drm/i915/sdvo: Grab mode_config.mutex during LVDS init to avoid WARNs
       [not found] <20221026101134.20865-1-ville.syrjala@linux.intel.com>
  2022-10-26 10:11 ` [PATCH 1/8] drm/i915/sdvo: Filter out invalid outputs more sensibly Ville Syrjala
  2022-10-26 10:11 ` [PATCH 2/8] drm/i915/sdvo: Setup DDC fully before output init Ville Syrjala
@ 2022-10-26 10:11 ` Ville Syrjala
  2022-10-27 14:37   ` [Intel-gfx] " Jani Nikula
  2 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjala @ 2022-10-26 10:11 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_mode_probed_add() is unhappy about being called w/o
mode_config.mutex. Grab it during LVDS fixed mode setup
to silence the WARNs.

Cc: stable@vger.kernel.org
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
Fixes: aa2b88074a56 ("drm/i915/sdvo: Fix multi function encoder stuff")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_sdvo.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index ccf81d616cb4..1eaaa7ec580e 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2899,8 +2899,12 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
 	intel_panel_add_vbt_sdvo_fixed_mode(intel_connector);
 
 	if (!intel_panel_preferred_fixed_mode(intel_connector)) {
+		mutex_lock(&i915->drm.mode_config.mutex);
+
 		intel_ddc_get_modes(connector, &intel_sdvo->ddc);
 		intel_panel_add_edid_fixed_modes(intel_connector, false);
+
+		mutex_unlock(&i915->drm.mode_config.mutex);
 	}
 
 	intel_panel_init(intel_connector);
-- 
2.37.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/8] drm/i915/sdvo: Filter out invalid outputs more sensibly
  2022-10-26 10:11 ` [PATCH 1/8] drm/i915/sdvo: Filter out invalid outputs more sensibly Ville Syrjala
@ 2022-10-27 14:32   ` Jani Nikula
  0 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2022-10-27 14:32 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: stable

On Wed, 26 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We try to filter out the corresponding xxx1 output
> if the xxx0 output is not present. But the way that is
> being done is pretty awkward. Make it less so.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_sdvo.c | 29 ++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index cf8e80936d8e..c6200a91a777 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2925,16 +2925,33 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
>  	return false;
>  }
>  
> -static bool
> -intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
> +static u16 intel_sdvo_filter_output_flags(u16 flags)
>  {
> +	flags &= SDVO_OUTPUT_MASK;
> +
>  	/* SDVO requires XXX1 function may not exist unless it has XXX0 function.*/
> +	if (!(flags & SDVO_OUTPUT_TMDS0))
> +		flags &= ~SDVO_OUTPUT_TMDS1;
> +
> +	if (!(flags & SDVO_OUTPUT_RGB0))
> +		flags &= ~SDVO_OUTPUT_RGB1;
> +
> +	if (!(flags & SDVO_OUTPUT_LVDS0))
> +		flags &= ~SDVO_OUTPUT_LVDS1;
> +
> +	return flags;
> +}
> +
> +static bool
> +intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
> +{
> +	flags = intel_sdvo_filter_output_flags(flags);
>  
>  	if (flags & SDVO_OUTPUT_TMDS0)
>  		if (!intel_sdvo_dvi_init(intel_sdvo, 0))
>  			return false;
>  
> -	if ((flags & SDVO_TMDS_MASK) == SDVO_TMDS_MASK)
> +	if (flags & SDVO_OUTPUT_TMDS1)
>  		if (!intel_sdvo_dvi_init(intel_sdvo, 1))
>  			return false;
>  
> @@ -2955,7 +2972,7 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
>  		if (!intel_sdvo_analog_init(intel_sdvo, 0))
>  			return false;
>  
> -	if ((flags & SDVO_RGB_MASK) == SDVO_RGB_MASK)
> +	if (flags & SDVO_OUTPUT_RGB1)
>  		if (!intel_sdvo_analog_init(intel_sdvo, 1))
>  			return false;
>  
> @@ -2963,11 +2980,11 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
>  		if (!intel_sdvo_lvds_init(intel_sdvo, 0))
>  			return false;
>  
> -	if ((flags & SDVO_LVDS_MASK) == SDVO_LVDS_MASK)
> +	if (flags & SDVO_OUTPUT_LVDS1)
>  		if (!intel_sdvo_lvds_init(intel_sdvo, 1))
>  			return false;
>  
> -	if ((flags & SDVO_OUTPUT_MASK) == 0) {
> +	if (flags == 0) {
>  		unsigned char bytes[2];
>  
>  		intel_sdvo->controlled_output = 0;

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/8] drm/i915/sdvo: Setup DDC fully before output init
  2022-10-26 10:11 ` [PATCH 2/8] drm/i915/sdvo: Setup DDC fully before output init Ville Syrjala
@ 2022-10-27 14:36   ` Jani Nikula
  2022-10-27 14:49     ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2022-10-27 14:36 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: stable

On Wed, 26 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Call intel_sdvo_select_ddc_bus() before initializing any
> of the outputs. And before that is functional (assuming no VBT)
> we have to set up the controlled_outputs thing. Otherwise DDC
> won't be functional during the output init but LVDS really
> needs it for the fixed mode setup.
>
> Note that the whole multi output support still looks very
> bogus, and more work will be needed to make it correct.
> But for now this should at least fix the LVDS EDID fixed mode
> setup.
>
> Cc: stable@vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
> Fixes: aa2b88074a56 ("drm/i915/sdvo: Fix multi function encoder stuff")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_sdvo.c | 31 +++++++++--------------
>  1 file changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index c6200a91a777..ccf81d616cb4 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2746,13 +2746,10 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>  	if (!intel_sdvo_connector)
>  		return false;
>  
> -	if (device == 0) {
> -		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0;
> +	if (device == 0)
>  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS0;
> -	} else if (device == 1) {
> -		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1;
> +	else if (device == 1)
>  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS1;
> -	}
>  
>  	intel_connector = &intel_sdvo_connector->base;
>  	connector = &intel_connector->base;
> @@ -2807,7 +2804,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
>  	encoder->encoder_type = DRM_MODE_ENCODER_TVDAC;
>  	connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO;
>  
> -	intel_sdvo->controlled_output |= type;
>  	intel_sdvo_connector->output_flag = type;
>  
>  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> @@ -2848,13 +2844,10 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
>  	encoder->encoder_type = DRM_MODE_ENCODER_DAC;
>  	connector->connector_type = DRM_MODE_CONNECTOR_VGA;
>  
> -	if (device == 0) {
> -		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0;
> +	if (device == 0)
>  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB0;
> -	} else if (device == 1) {
> -		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1;
> +	else if (device == 1)
>  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB1;
> -	}
>  
>  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
>  		kfree(intel_sdvo_connector);
> @@ -2884,13 +2877,10 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
>  	encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
>  	connector->connector_type = DRM_MODE_CONNECTOR_LVDS;
>  
> -	if (device == 0) {
> -		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0;
> +	if (device == 0)
>  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS0;
> -	} else if (device == 1) {
> -		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1;
> +	else if (device == 1)
>  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS1;
> -	}
>  
>  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
>  		kfree(intel_sdvo_connector);
> @@ -2945,8 +2935,14 @@ static u16 intel_sdvo_filter_output_flags(u16 flags)
>  static bool
>  intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
>  {
> +	struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
> +
>  	flags = intel_sdvo_filter_output_flags(flags);
>  
> +	intel_sdvo->controlled_output = flags;
> +
> +	intel_sdvo_select_ddc_bus(i915, intel_sdvo);

AFAICT the ->controlled_outputs member could now be removed and just
passed by value here.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +
>  	if (flags & SDVO_OUTPUT_TMDS0)
>  		if (!intel_sdvo_dvi_init(intel_sdvo, 0))
>  			return false;
> @@ -2987,7 +2983,6 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
>  	if (flags == 0) {
>  		unsigned char bytes[2];
>  
> -		intel_sdvo->controlled_output = 0;
>  		memcpy(bytes, &intel_sdvo->caps.output_flags, 2);
>  		DRM_DEBUG_KMS("%s: Unknown SDVO output type (0x%02x%02x)\n",
>  			      SDVO_NAME(intel_sdvo),
> @@ -3399,8 +3394,6 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
>  	 */
>  	intel_sdvo->base.cloneable = 0;
>  
> -	intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo);
> -
>  	/* Set the input timing to the screen. Assume always input 0. */
>  	if (!intel_sdvo_set_target_input(intel_sdvo))
>  		goto err_output;

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 3/8] drm/i915/sdvo: Grab mode_config.mutex during LVDS init to avoid WARNs
  2022-10-26 10:11 ` [PATCH 3/8] drm/i915/sdvo: Grab mode_config.mutex during LVDS init to avoid WARNs Ville Syrjala
@ 2022-10-27 14:37   ` Jani Nikula
  0 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2022-10-27 14:37 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: stable

On Wed, 26 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drm_mode_probed_add() is unhappy about being called w/o
> mode_config.mutex. Grab it during LVDS fixed mode setup
> to silence the WARNs.
>
> Cc: stable@vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
> Fixes: aa2b88074a56 ("drm/i915/sdvo: Fix multi function encoder stuff")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_sdvo.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index ccf81d616cb4..1eaaa7ec580e 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -2899,8 +2899,12 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
>  	intel_panel_add_vbt_sdvo_fixed_mode(intel_connector);
>  
>  	if (!intel_panel_preferred_fixed_mode(intel_connector)) {
> +		mutex_lock(&i915->drm.mode_config.mutex);
> +
>  		intel_ddc_get_modes(connector, &intel_sdvo->ddc);
>  		intel_panel_add_edid_fixed_modes(intel_connector, false);
> +
> +		mutex_unlock(&i915->drm.mode_config.mutex);
>  	}
>  
>  	intel_panel_init(intel_connector);

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/8] drm/i915/sdvo: Setup DDC fully before output init
  2022-10-27 14:36   ` [Intel-gfx] " Jani Nikula
@ 2022-10-27 14:49     ` Ville Syrjälä
  2022-10-27 14:54       ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2022-10-27 14:49 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, stable

On Thu, Oct 27, 2022 at 05:36:24PM +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Call intel_sdvo_select_ddc_bus() before initializing any
> > of the outputs. And before that is functional (assuming no VBT)
> > we have to set up the controlled_outputs thing. Otherwise DDC
> > won't be functional during the output init but LVDS really
> > needs it for the fixed mode setup.
> >
> > Note that the whole multi output support still looks very
> > bogus, and more work will be needed to make it correct.
> > But for now this should at least fix the LVDS EDID fixed mode
> > setup.
> >
> > Cc: stable@vger.kernel.org
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
> > Fixes: aa2b88074a56 ("drm/i915/sdvo: Fix multi function encoder stuff")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_sdvo.c | 31 +++++++++--------------
> >  1 file changed, 12 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > index c6200a91a777..ccf81d616cb4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > @@ -2746,13 +2746,10 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> >  	if (!intel_sdvo_connector)
> >  		return false;
> >  
> > -	if (device == 0) {
> > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0;
> > +	if (device == 0)
> >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS0;
> > -	} else if (device == 1) {
> > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1;
> > +	else if (device == 1)
> >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS1;
> > -	}
> >  
> >  	intel_connector = &intel_sdvo_connector->base;
> >  	connector = &intel_connector->base;
> > @@ -2807,7 +2804,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
> >  	encoder->encoder_type = DRM_MODE_ENCODER_TVDAC;
> >  	connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO;
> >  
> > -	intel_sdvo->controlled_output |= type;
> >  	intel_sdvo_connector->output_flag = type;
> >  
> >  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> > @@ -2848,13 +2844,10 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
> >  	encoder->encoder_type = DRM_MODE_ENCODER_DAC;
> >  	connector->connector_type = DRM_MODE_CONNECTOR_VGA;
> >  
> > -	if (device == 0) {
> > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0;
> > +	if (device == 0)
> >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB0;
> > -	} else if (device == 1) {
> > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1;
> > +	else if (device == 1)
> >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB1;
> > -	}
> >  
> >  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> >  		kfree(intel_sdvo_connector);
> > @@ -2884,13 +2877,10 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
> >  	encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
> >  	connector->connector_type = DRM_MODE_CONNECTOR_LVDS;
> >  
> > -	if (device == 0) {
> > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0;
> > +	if (device == 0)
> >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS0;
> > -	} else if (device == 1) {
> > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1;
> > +	else if (device == 1)
> >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS1;
> > -	}
> >  
> >  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> >  		kfree(intel_sdvo_connector);
> > @@ -2945,8 +2935,14 @@ static u16 intel_sdvo_filter_output_flags(u16 flags)
> >  static bool
> >  intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
> >  {
> > +	struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
> > +
> >  	flags = intel_sdvo_filter_output_flags(flags);
> >  
> > +	intel_sdvo->controlled_output = flags;
> > +
> > +	intel_sdvo_select_ddc_bus(i915, intel_sdvo);
> 
> AFAICT the ->controlled_outputs member could now be removed and just
> passed by value here.

Hmm. True. Though the whole thing is utter garbage anyway.
intel_sdvo_guess_ddc_bus() really expects controlled_outputs
to contain only a single bit. I guess it kinda works by luck
most or the time, at least for single output devices.
I suppose I can still include the controlled_outputs nukage
into this patch.

What we really need to do is to have a ddc adapter per each
connector, and then each one can properly figure out its
own ddc bus. Right now there is some kind of horrible hack
in intel_sdvo_tmds_sink_detect() that probes all the ddc
buses if it didn't find anything on the one that got
chosen initally. 

Fixing intel_sdvo_guess_ddc_bus() for that is trivial, but
I think fixing the VBT mapping stuff is a bit harder since
we now just parse that per SDVO port rather than per SDVO
output.

attached_output is another nonsense thing that needs to
get nuked, but that one should be a bit easier. Anyways,
more work clearly needed here...

> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Thanks.

> 
> 
> > +
> >  	if (flags & SDVO_OUTPUT_TMDS0)
> >  		if (!intel_sdvo_dvi_init(intel_sdvo, 0))
> >  			return false;
> > @@ -2987,7 +2983,6 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
> >  	if (flags == 0) {
> >  		unsigned char bytes[2];
> >  
> > -		intel_sdvo->controlled_output = 0;
> >  		memcpy(bytes, &intel_sdvo->caps.output_flags, 2);
> >  		DRM_DEBUG_KMS("%s: Unknown SDVO output type (0x%02x%02x)\n",
> >  			      SDVO_NAME(intel_sdvo),
> > @@ -3399,8 +3394,6 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
> >  	 */
> >  	intel_sdvo->base.cloneable = 0;
> >  
> > -	intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo);
> > -
> >  	/* Set the input timing to the screen. Assume always input 0. */
> >  	if (!intel_sdvo_set_target_input(intel_sdvo))
> >  		goto err_output;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/8] drm/i915/sdvo: Setup DDC fully before output init
  2022-10-27 14:49     ` Ville Syrjälä
@ 2022-10-27 14:54       ` Ville Syrjälä
  2022-10-27 17:33         ` Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2022-10-27 14:54 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, stable

On Thu, Oct 27, 2022 at 05:49:53PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 27, 2022 at 05:36:24PM +0300, Jani Nikula wrote:
> > On Wed, 26 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Call intel_sdvo_select_ddc_bus() before initializing any
> > > of the outputs. And before that is functional (assuming no VBT)
> > > we have to set up the controlled_outputs thing. Otherwise DDC
> > > won't be functional during the output init but LVDS really
> > > needs it for the fixed mode setup.
> > >
> > > Note that the whole multi output support still looks very
> > > bogus, and more work will be needed to make it correct.
> > > But for now this should at least fix the LVDS EDID fixed mode
> > > setup.
> > >
> > > Cc: stable@vger.kernel.org
> > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
> > > Fixes: aa2b88074a56 ("drm/i915/sdvo: Fix multi function encoder stuff")
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_sdvo.c | 31 +++++++++--------------
> > >  1 file changed, 12 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > > index c6200a91a777..ccf81d616cb4 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > > @@ -2746,13 +2746,10 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> > >  	if (!intel_sdvo_connector)
> > >  		return false;
> > >  
> > > -	if (device == 0) {
> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0;
> > > +	if (device == 0)
> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS0;
> > > -	} else if (device == 1) {
> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1;
> > > +	else if (device == 1)
> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS1;
> > > -	}
> > >  
> > >  	intel_connector = &intel_sdvo_connector->base;
> > >  	connector = &intel_connector->base;
> > > @@ -2807,7 +2804,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
> > >  	encoder->encoder_type = DRM_MODE_ENCODER_TVDAC;
> > >  	connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO;
> > >  
> > > -	intel_sdvo->controlled_output |= type;
> > >  	intel_sdvo_connector->output_flag = type;
> > >  
> > >  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> > > @@ -2848,13 +2844,10 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
> > >  	encoder->encoder_type = DRM_MODE_ENCODER_DAC;
> > >  	connector->connector_type = DRM_MODE_CONNECTOR_VGA;
> > >  
> > > -	if (device == 0) {
> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0;
> > > +	if (device == 0)
> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB0;
> > > -	} else if (device == 1) {
> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1;
> > > +	else if (device == 1)
> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB1;
> > > -	}
> > >  
> > >  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> > >  		kfree(intel_sdvo_connector);
> > > @@ -2884,13 +2877,10 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
> > >  	encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
> > >  	connector->connector_type = DRM_MODE_CONNECTOR_LVDS;
> > >  
> > > -	if (device == 0) {
> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0;
> > > +	if (device == 0)
> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS0;
> > > -	} else if (device == 1) {
> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1;
> > > +	else if (device == 1)
> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS1;
> > > -	}
> > >  
> > >  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
> > >  		kfree(intel_sdvo_connector);
> > > @@ -2945,8 +2935,14 @@ static u16 intel_sdvo_filter_output_flags(u16 flags)
> > >  static bool
> > >  intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
> > >  {
> > > +	struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
> > > +
> > >  	flags = intel_sdvo_filter_output_flags(flags);
> > >  
> > > +	intel_sdvo->controlled_output = flags;
> > > +
> > > +	intel_sdvo_select_ddc_bus(i915, intel_sdvo);
> > 
> > AFAICT the ->controlled_outputs member could now be removed and just
> > passed by value here.
> 
> Hmm. True. Though the whole thing is utter garbage anyway.
> intel_sdvo_guess_ddc_bus() really expects controlled_outputs
> to contain only a single bit. I guess it kinda works by luck
> most or the time, at least for single output devices.
> I suppose I can still include the controlled_outputs nukage
> into this patch.

On second though I think I'll do it as a followup. Less chance
of backport conflicts that way.

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/8] drm/i915/sdvo: Setup DDC fully before output init
  2022-10-27 14:54       ` Ville Syrjälä
@ 2022-10-27 17:33         ` Jani Nikula
  0 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2022-10-27 17:33 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, stable

On Thu, 27 Oct 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Oct 27, 2022 at 05:49:53PM +0300, Ville Syrjälä wrote:
>> On Thu, Oct 27, 2022 at 05:36:24PM +0300, Jani Nikula wrote:
>> > On Wed, 26 Oct 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > >
>> > > Call intel_sdvo_select_ddc_bus() before initializing any
>> > > of the outputs. And before that is functional (assuming no VBT)
>> > > we have to set up the controlled_outputs thing. Otherwise DDC
>> > > won't be functional during the output init but LVDS really
>> > > needs it for the fixed mode setup.
>> > >
>> > > Note that the whole multi output support still looks very
>> > > bogus, and more work will be needed to make it correct.
>> > > But for now this should at least fix the LVDS EDID fixed mode
>> > > setup.
>> > >
>> > > Cc: stable@vger.kernel.org
>> > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7301
>> > > Fixes: aa2b88074a56 ("drm/i915/sdvo: Fix multi function encoder stuff")
>> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/display/intel_sdvo.c | 31 +++++++++--------------
>> > >  1 file changed, 12 insertions(+), 19 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > > index c6200a91a777..ccf81d616cb4 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > > @@ -2746,13 +2746,10 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>> > >  	if (!intel_sdvo_connector)
>> > >  		return false;
>> > >  
>> > > -	if (device == 0) {
>> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0;
>> > > +	if (device == 0)
>> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS0;
>> > > -	} else if (device == 1) {
>> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1;
>> > > +	else if (device == 1)
>> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_TMDS1;
>> > > -	}
>> > >  
>> > >  	intel_connector = &intel_sdvo_connector->base;
>> > >  	connector = &intel_connector->base;
>> > > @@ -2807,7 +2804,6 @@ intel_sdvo_tv_init(struct intel_sdvo *intel_sdvo, int type)
>> > >  	encoder->encoder_type = DRM_MODE_ENCODER_TVDAC;
>> > >  	connector->connector_type = DRM_MODE_CONNECTOR_SVIDEO;
>> > >  
>> > > -	intel_sdvo->controlled_output |= type;
>> > >  	intel_sdvo_connector->output_flag = type;
>> > >  
>> > >  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
>> > > @@ -2848,13 +2844,10 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
>> > >  	encoder->encoder_type = DRM_MODE_ENCODER_DAC;
>> > >  	connector->connector_type = DRM_MODE_CONNECTOR_VGA;
>> > >  
>> > > -	if (device == 0) {
>> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0;
>> > > +	if (device == 0)
>> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB0;
>> > > -	} else if (device == 1) {
>> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1;
>> > > +	else if (device == 1)
>> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_RGB1;
>> > > -	}
>> > >  
>> > >  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
>> > >  		kfree(intel_sdvo_connector);
>> > > @@ -2884,13 +2877,10 @@ intel_sdvo_lvds_init(struct intel_sdvo *intel_sdvo, int device)
>> > >  	encoder->encoder_type = DRM_MODE_ENCODER_LVDS;
>> > >  	connector->connector_type = DRM_MODE_CONNECTOR_LVDS;
>> > >  
>> > > -	if (device == 0) {
>> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0;
>> > > +	if (device == 0)
>> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS0;
>> > > -	} else if (device == 1) {
>> > > -		intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1;
>> > > +	else if (device == 1)
>> > >  		intel_sdvo_connector->output_flag = SDVO_OUTPUT_LVDS1;
>> > > -	}
>> > >  
>> > >  	if (intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo) < 0) {
>> > >  		kfree(intel_sdvo_connector);
>> > > @@ -2945,8 +2935,14 @@ static u16 intel_sdvo_filter_output_flags(u16 flags)
>> > >  static bool
>> > >  intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
>> > >  {
>> > > +	struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
>> > > +
>> > >  	flags = intel_sdvo_filter_output_flags(flags);
>> > >  
>> > > +	intel_sdvo->controlled_output = flags;
>> > > +
>> > > +	intel_sdvo_select_ddc_bus(i915, intel_sdvo);
>> > 
>> > AFAICT the ->controlled_outputs member could now be removed and just
>> > passed by value here.
>> 
>> Hmm. True. Though the whole thing is utter garbage anyway.
>> intel_sdvo_guess_ddc_bus() really expects controlled_outputs
>> to contain only a single bit. I guess it kinda works by luck
>> most or the time, at least for single output devices.
>> I suppose I can still include the controlled_outputs nukage
>> into this patch.
>
> On second though I think I'll do it as a followup. Less chance
> of backport conflicts that way.

That's completely fine, it was just an observation.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-10-27 17:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20221026101134.20865-1-ville.syrjala@linux.intel.com>
2022-10-26 10:11 ` [PATCH 1/8] drm/i915/sdvo: Filter out invalid outputs more sensibly Ville Syrjala
2022-10-27 14:32   ` [Intel-gfx] " Jani Nikula
2022-10-26 10:11 ` [PATCH 2/8] drm/i915/sdvo: Setup DDC fully before output init Ville Syrjala
2022-10-27 14:36   ` [Intel-gfx] " Jani Nikula
2022-10-27 14:49     ` Ville Syrjälä
2022-10-27 14:54       ` Ville Syrjälä
2022-10-27 17:33         ` Jani Nikula
2022-10-26 10:11 ` [PATCH 3/8] drm/i915/sdvo: Grab mode_config.mutex during LVDS init to avoid WARNs Ville Syrjala
2022-10-27 14:37   ` [Intel-gfx] " Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).