| Message ID | 20190619180312.31817-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] drm/i915/sdvo: Fix handling if zero hbuf size | expand |
On Wed, Jun 19, 2019 at 09:03:07PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@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.syrjala@linux.intel.com> Matches the spec: Reviewed-by: Imre Deak <imre.deak@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
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);