On Wed, Jun 19, 2019 at 09:03:07PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> The spec says:
> "A value of 0 indicates that this buffer does not exist"
> So we should not convert a hbuf_size of 0 into 1.
> 
> Also pull the relevant code into a helper to avoid making the
> same mistake multiple times.
> 
> And while at it fix the debug prints to not say "hbuf_len" twice.
> 
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

Matches the spec:
Reviewed-by: Imre Deak <imre.d...@intel.com>

Noticed the %i vs. %u usage in the debug message, but it doesn't seem to
cause any problem.

> ---
>  drivers/gpu/drm/i915/display/intel_sdvo.c | 32 ++++++++++++++---------
>  1 file changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c 
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index ceda03e5a3d4..681411aae754 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -929,6 +929,20 @@ static bool intel_sdvo_set_audio_state(struct intel_sdvo 
> *intel_sdvo,
>                                   &audio_state, 1);
>  }
>  
> +static bool intel_sdvo_get_hbuf_size(struct intel_sdvo *intel_sdvo,
> +                                  u8 *hbuf_size)
> +{
> +     if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_HBUF_INFO,
> +                               hbuf_size, 1))
> +             return false;
> +
> +     /* Buffer size is 0 based, hooray! However zero means zero. */
> +     if (*hbuf_size)
> +             (*hbuf_size)++;
> +
> +     return true;
> +}
> +
>  #if 0
>  static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
>  {
> @@ -972,14 +986,10 @@ static bool intel_sdvo_write_infoframe(struct 
> intel_sdvo *intel_sdvo,
>                                 set_buf_index, 2))
>               return false;
>  
> -     if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_HBUF_INFO,
> -                               &hbuf_size, 1))
> +     if (!intel_sdvo_get_hbuf_size(intel_sdvo, &hbuf_size))
>               return false;
>  
> -     /* Buffer size is 0 based, hooray! */
> -     hbuf_size++;
> -
> -     DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size: %i\n",
> +     DRM_DEBUG_KMS("writing sdvo hbuf: %i, length %i, hbuf_size: %i\n",
>                     if_index, length, hbuf_size);
>  
>       if (hbuf_size < length)
> @@ -1030,14 +1040,10 @@ static ssize_t intel_sdvo_read_infoframe(struct 
> intel_sdvo *intel_sdvo,
>       if (tx_rate == SDVO_HBUF_TX_DISABLED)
>               return 0;
>  
> -     if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_HBUF_INFO,
> -                               &hbuf_size, 1))
> -             return -ENXIO;
> -
> -     /* Buffer size is 0 based, hooray! */
> -     hbuf_size++;
> +     if (!intel_sdvo_get_hbuf_size(intel_sdvo, &hbuf_size))
> +             return false;
>  
> -     DRM_DEBUG_KMS("reading sdvo hbuf: %i, hbuf_size %i, hbuf_size: %i\n",
> +     DRM_DEBUG_KMS("reading sdvo hbuf: %i, length %i, hbuf_size: %i\n",
>                     if_index, length, hbuf_size);
>  
>       hbuf_size = min_t(unsigned int, length, hbuf_size);
> -- 
> 2.21.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to