Re: [Intel-gfx] [PATCH v2 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Sripada, Radhakrishna]

On Tue, 2019-04-23 at 17:34 +0300, Ville Syrjälä wrote:
> On Mon, Apr 22, 2019 at 06:37:48PM +, Sripada, Radhakrishna
> wrote:
> > On Wed, 2019-04-10 at 20:08 +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > > 
> > > Before we go writing the infoframe let's make sure we have
> > > the space for it. Not that it really matters since the write
> > > loop would just terminate early in that case.
> > > 
> > > v2: Check after the debug print and ++ (Chris)
> > > 
> > > Signed-off-by: Ville Syrjälä 
> > > Reviewed-by: Chris Wilson 
> > > ---
> > >  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > > b/drivers/gpu/drm/i915/intel_sdvo.c
> > > index 7f64352a3413..14348dfb024a 100644
> > > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > > @@ -976,6 +976,9 @@ static bool intel_sdvo_write_infoframe(struct
> > > intel_sdvo *intel_sdvo,
> > >   DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size:
> > > %i\n",
> > > if_index, length, hbuf_size);
> > 
> > If the above line is newly added why dont I See a + at the
> > beginning of
> > the line? Is it from a previous version of the patch?
> 
> It's not newly added. What gave you the idea that it was?
NVM. I was comparing the raw diffs between v1 and v2 of the patch and
missed the above line in the context lines in v1 of the patch. Just a
minor code move around. Will be cautious in future reviews.

LGTM.
Reviewed-by: Radhakrishna Sripada 
> 
> > 
> > -Radhakrishna(RK) Sripada
> > >  
> > > + if (hbuf_size < length)
> > > + return false;
> > > +
> > >   for (i = 0; i < hbuf_size; i += 8) {
> > >   memset(tmp, 0, 8);
> > >   if (i < length)
> 
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Ville Syrjälä]

On Mon, Apr 22, 2019 at 06:37:48PM +, Sripada, Radhakrishna wrote:
> On Wed, 2019-04-10 at 20:08 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > Before we go writing the infoframe let's make sure we have
> > the space for it. Not that it really matters since the write
> > loop would just terminate early in that case.
> > 
> > v2: Check after the debug print and ++ (Chris)
> > 
> > Signed-off-by: Ville Syrjälä 
> > Reviewed-by: Chris Wilson 
> > ---
> >  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 7f64352a3413..14348dfb024a 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -976,6 +976,9 @@ static bool intel_sdvo_write_infoframe(struct
> > intel_sdvo *intel_sdvo,
> > DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size:
> > %i\n",
> >   if_index, length, hbuf_size);
> If the above line is newly added why dont I See a + at the beginning of
> the line? Is it from a previous version of the patch?

It's not newly added. What gave you the idea that it was?

> 
> -Radhakrishna(RK) Sripada
> >  
> > +   if (hbuf_size < length)
> > +   return false;
> > +
> > for (i = 0; i < hbuf_size; i += 8) {
> > memset(tmp, 0, 8);
> > if (i < length)

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Sripada, Radhakrishna]

On Wed, 2019-04-10 at 20:08 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Before we go writing the infoframe let's make sure we have
> the space for it. Not that it really matters since the write
> loop would just terminate early in that case.
> 
> v2: Check after the debug print and ++ (Chris)
> 
> Signed-off-by: Ville Syrjälä 
> Reviewed-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index 7f64352a3413..14348dfb024a 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -976,6 +976,9 @@ static bool intel_sdvo_write_infoframe(struct
> intel_sdvo *intel_sdvo,
>   DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size:
> %i\n",
> if_index, length, hbuf_size);
If the above line is newly added why dont I See a + at the beginning of
the line? Is it from a previous version of the patch?

-Radhakrishna(RK) Sripada
>  
> + if (hbuf_size < length)
> + return false;
> +
>   for (i = 0; i < hbuf_size; i += 8) {
>   memset(tmp, 0, 8);
>   if (i < length)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Ville Syrjala]

From: Ville Syrjälä 

Before we go writing the infoframe let's make sure we have
the space for it. Not that it really matters since the write
loop would just terminate early in that case.

v2: Check after the debug print and ++ (Chris)

Signed-off-by: Ville Syrjälä 
Reviewed-by: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c 
b/drivers/gpu/drm/i915/intel_sdvo.c
index 7f64352a3413..14348dfb024a 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -976,6 +976,9 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo 
*intel_sdvo,
DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size: %i\n",
  if_index, length, hbuf_size);
 
+   if (hbuf_size < length)
+   return false;
+
for (i = 0; i < hbuf_size; i += 8) {
memset(tmp, 0, 8);
if (i < length)
-- 
2.21.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx