Re: [PATCH 6/6] drm/i915/sdvo: Add helpers to get the cmd/status string

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 6/6] drm/i915/sdvo: Add helpers to get the cmd/status string
Date: Wed, 19 Jun 2019 19:15:55 +0100	[thread overview]
Message-ID: <156096815595.664.4978579433721792478@skylake-alporthouse-com> (raw)
In-Reply-To: <20190619180312.31817-6-ville.syrjala@linux.intel.com>

Quoting Ville Syrjala (2019-06-19 19:03:12)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add sdvo_cmd_name() and sdvo_cmd_status() helpers to avoid bothering
> the callers with the implementation details of the storage for these
> strings.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_sdvo.c | 40 +++++++++++++++++------
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index df3582bab076..184cd64a7f98 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -395,11 +395,24 @@ static const struct {
>  
>  #undef SDVO_CMD_NAME_ENTRY
>  
> +static const char *sdvo_cmd_name(u8 cmd)
> +{
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
> +               if (cmd == sdvo_cmd_names[i].cmd)
> +                       return sdvo_cmd_names[i].name;
> +       }
> +
> +       return NULL;
> +}
> +
>  #define SDVO_NAME(svdo) ((svdo)->port == PORT_B ? "SDVOB" : "SDVOC")
>  
>  static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
>                                    const void *args, int args_len)
>  {
> +       const char *cmd_name;
>         int i, pos = 0;
>         char buffer[64];
>  
> @@ -412,15 +425,12 @@ static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
>         for (; i < 8; i++) {
>                 BUF_PRINT("   ");
>         }
> -       for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
> -               if (cmd == sdvo_cmd_names[i].cmd) {
> -                       BUF_PRINT("(%s)", sdvo_cmd_names[i].name);
> -                       break;
> -               }
> -       }
> -       if (i == ARRAY_SIZE(sdvo_cmd_names)) {
> +
> +       cmd_name = sdvo_cmd_name(cmd);
> +       if (cmd_name)
> +               BUF_PRINT("(%s)", cmd_name);
> +       else
>                 BUF_PRINT("(%02X)", cmd);
> -       }
>  
>         WARN_ON(pos >= sizeof(buffer) - 1);
>  #undef BUF_PRINT
> @@ -438,6 +448,14 @@ static const char * const cmd_status_names[] = {
>         [SDVO_CMD_STATUS_SCALING_NOT_SUPP] = "Scaling not supported",
>  };
>  
> +static const char *sdvo_cmd_status(u8 status)
> +{
> +       if (status < ARRAY_SIZE(cmd_status_names))
> +               return cmd_status_names[status];
> +       else
> +               return NULL;
> +}
> +
>  static bool __intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd,
>                                    const void *args, int args_len,
>                                    bool unlocked)
> @@ -516,6 +534,7 @@ static bool intel_sdvo_write_cmd(struct intel_sdvo *intel_sdvo, u8 cmd,
>  static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>                                      void *response, int response_len)
>  {
> +       const char *cmd_status;
>         u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
>         u8 status;
>         int i, pos = 0;
> @@ -561,8 +580,9 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>  #define BUF_PRINT(args...) \
>         pos += snprintf(buffer + pos, max_t(int, sizeof(buffer) - pos, 0), args)
>  
> -       if (status < ARRAY_SIZE(cmd_status_names))
> -               BUF_PRINT("(%s)", cmd_status_names[status]);
> +       cmd_status = sdvo_cmd_status(status);
> +       if (cmd_status)
> +               BUF_PRINT("(%s)", cmd_status);

There's my earlier suggestion!

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx