[Intel-gfx] [PATCH] drm/i915: clear the entire sdvo infoframe buffer

[Ben Hutchings]

On Sat, 2012-10-20 at 18:33 +0200, Daniel Vetter wrote:
> Like in the case of native hdmi, which is fixed already in
> 
> commit adf00b26d18e1b3570451296e03bcb20e4798cdd
> Author: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Date:   Tue Sep 25 13:23:34 2012 -0300
> 
>     drm/i915: make sure we write all the DIP data bytes
> 
> we need to clear the entire sdvo buffer to avoid upsetting the
> display.
> 
> Since infoframe buffer writing is now a bit more elaborate, extract it
> into it's own function. This will be useful if we ever get around to
> properly update the ELD for sdvo. Also #define proper names for the
> two buffer indexes with fixed usage.
> 
> v2: Cite the right commit above, spotted by Paulo Zanoni.
> 
> v3: I'm too stupid to paste the right commit.
> 
> Reported-and-tested-by: Jürg Billeter <j at bitron.ch>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=25732
> Cc: stable at vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c      |   64 ++++++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_sdvo_regs.h |    2 +
>  2 files changed, 46 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index d2e8c9f..4bc9c52 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -895,6 +895,47 @@ static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
>  }
>  #endif
>  
> +static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
> +				       unsigned if_index, uint8_t tx_rate,
> +				       uint8_t *data, unsigned length)
> +{
> +	uint8_t set_buf_index[2] = { if_index, 0 };
> +	uint8_t hbuf_size, tmp[8];
> +	int i;
> +
> +	if (!intel_sdvo_set_value(intel_sdvo,
> +				  SDVO_CMD_SET_HBUF_INDEX,
> +				  set_buf_index, 2))
> +		return false;
> +
> +	if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_HBUF_INFO,
> +				  &hbuf_size, 1))
> +		return false;
> +
> +	/* Buffer size is 0 base, hooray! */
> +	hbuf_size++;

So SDVO_CMD_GET_HBUF_INFO gave us the buffer size in bytes, minus 1?

> +	DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size: %i\n",
> +		      if_index, length, hbuf_size);
> +
> +	for (i = 0; i < hbuf_size; i += 8) {
> +		memset(tmp, 0, 8);
> +		memcpy(tmp, data, min_t(unsigned, 8, length));
> +
> +		if (!intel_sdvo_set_value(intel_sdvo,
> +					  SDVO_CMD_SET_HBUF_DATA,
> +					  tmp, 8))
> +			return false;
> +
> +		data += 8;
> +		length -= 8;
> +	}
[...]

Based on the commit description, I would assume that hbuf_size may be
greater than length initialy, and you're trying to zero-pad here.  But
if hbuf_size >= length + 8 initially then length can wrap around and
this will continue to read beyond the end of the data array.

I think you could leave data and length unchanged in the loop and then
set up each data chunk like this:

		memset(tmp, 0, 8);
		if (i < length)
			memcpy(tmp, data + i, min_t(unsigned, 8, length - i);

Ben.

-- 
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
                                                         - Carolyn Scheppner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 828 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20121021/53bb36b0/attachment.sig>