Re: [Intel-gfx] [PATCH 4/8] drm/i915/sdvo: Simplify output setup debugs

[Jani Nikula]

On Wed, 26 Oct 2022, Ville Syrjala  wrote:
> From: Ville Syrjälä 
>
> Get rid of this funny byte based dumping of invalid output
> flags and just dump it as a single hex numbers. Also do that
> early since all the rest is going to get skipped anyway of
> the thing is zero.
>
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Jani Nikula 

> ---
>  drivers/gpu/drm/i915/display/intel_sdvo.c | 25 ++-
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c 
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 1eaaa7ec580e..d432f70001b7 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -199,7 +199,7 @@ to_intel_sdvo_connector(struct drm_connector *connector)
>   container_of((conn_state), struct intel_sdvo_connector_state, base.base)
>  
>  static bool
> -intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags);
> +intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo);
>  static bool
>  intel_sdvo_tv_create_property(struct intel_sdvo *intel_sdvo,
> struct intel_sdvo_connector *intel_sdvo_connector,
> @@ -2937,11 +2937,18 @@ static u16 intel_sdvo_filter_output_flags(u16 flags)
>  }
>  
>  static bool
> -intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
> +intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo)
>  {
>   struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
> + u16 flags;
>  
> - flags = intel_sdvo_filter_output_flags(flags);
> + flags = intel_sdvo_filter_output_flags(intel_sdvo->caps.output_flags);
> +
> + if (flags == 0) {
> + DRM_DEBUG_KMS("%s: Unknown SDVO output type (0x%04x)\n",
> +   SDVO_NAME(intel_sdvo), 
> intel_sdvo->caps.output_flags);
> + return false;
> + }
>  
>   intel_sdvo->controlled_output = flags;
>  
> @@ -2984,15 +2991,6 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, 
> u16 flags)
>   if (!intel_sdvo_lvds_init(intel_sdvo, 1))
>   return false;
>  
> - if (flags == 0) {
> - unsigned char bytes[2];
> -
> - memcpy(bytes, &intel_sdvo->caps.output_flags, 2);
> - DRM_DEBUG_KMS("%s: Unknown SDVO output type (0x%02x%02x)\n",
> -   SDVO_NAME(intel_sdvo),
> -   bytes[0], bytes[1]);
> - return false;
> - }
>   intel_sdvo->base.pipe_mask = ~0;
>  
>   return true;
> @@ -3368,8 +3366,7 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
>   intel_sdvo->colorimetry_cap =
>   intel_sdvo_get_colorimetry_cap(intel_sdvo);
>  
> - if (intel_sdvo_output_setup(intel_sdvo,
> - intel_sdvo->caps.output_flags) != true) {
> + if (!intel_sdvo_output_setup(intel_sdvo)) {
>   drm_dbg_kms(&dev_priv->drm,
>   "SDVO output failed to setup on %s\n",
>   SDVO_NAME(intel_sdvo));

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [PATCH 4/8] drm/i915/sdvo: Simplify output setup debugs

[Ville Syrjala]

From: Ville Syrjälä 

Get rid of this funny byte based dumping of invalid output
flags and just dump it as a single hex numbers. Also do that
early since all the rest is going to get skipped anyway of
the thing is zero.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_sdvo.c | 25 ++-
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c 
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 1eaaa7ec580e..d432f70001b7 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -199,7 +199,7 @@ to_intel_sdvo_connector(struct drm_connector *connector)
container_of((conn_state), struct intel_sdvo_connector_state, base.base)
 
 static bool
-intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags);
+intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo);
 static bool
 intel_sdvo_tv_create_property(struct intel_sdvo *intel_sdvo,
  struct intel_sdvo_connector *intel_sdvo_connector,
@@ -2937,11 +2937,18 @@ static u16 intel_sdvo_filter_output_flags(u16 flags)
 }
 
 static bool
-intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, u16 flags)
+intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo)
 {
struct drm_i915_private *i915 = to_i915(intel_sdvo->base.base.dev);
+   u16 flags;
 
-   flags = intel_sdvo_filter_output_flags(flags);
+   flags = intel_sdvo_filter_output_flags(intel_sdvo->caps.output_flags);
+
+   if (flags == 0) {
+   DRM_DEBUG_KMS("%s: Unknown SDVO output type (0x%04x)\n",
+ SDVO_NAME(intel_sdvo), 
intel_sdvo->caps.output_flags);
+   return false;
+   }
 
intel_sdvo->controlled_output = flags;
 
@@ -2984,15 +2991,6 @@ intel_sdvo_output_setup(struct intel_sdvo *intel_sdvo, 
u16 flags)
if (!intel_sdvo_lvds_init(intel_sdvo, 1))
return false;
 
-   if (flags == 0) {
-   unsigned char bytes[2];
-
-   memcpy(bytes, &intel_sdvo->caps.output_flags, 2);
-   DRM_DEBUG_KMS("%s: Unknown SDVO output type (0x%02x%02x)\n",
- SDVO_NAME(intel_sdvo),
- bytes[0], bytes[1]);
-   return false;
-   }
intel_sdvo->base.pipe_mask = ~0;
 
return true;
@@ -3368,8 +3366,7 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
intel_sdvo->colorimetry_cap =
intel_sdvo_get_colorimetry_cap(intel_sdvo);
 
-   if (intel_sdvo_output_setup(intel_sdvo,
-   intel_sdvo->caps.output_flags) != true) {
+   if (!intel_sdvo_output_setup(intel_sdvo)) {
drm_dbg_kms(&dev_priv->drm,
"SDVO output failed to setup on %s\n",
SDVO_NAME(intel_sdvo));
-- 
2.37.4