Re: [Intel-gfx] [PATCH 1/8] drm/i915/sdvo: Filter out invalid outputs more sensibly

[Jani Nikula]

On Wed, 26 Oct 2022, Ville Syrjala  wrote:
> From: Ville Syrjälä 
>
> 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: sta...@vger.kernel.org
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Jani Nikula 

> ---
>  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

[Intel-gfx] [PATCH 1/8] drm/i915/sdvo: Filter out invalid outputs more sensibly

[Ville Syrjala]

From: Ville Syrjälä 

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: sta...@vger.kernel.org
Signed-off-by: Ville Syrjälä 
---
 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