On 12/06, Melissa Wen wrote:
> On 11/10, Arthur Grillo wrote:
> > 
> > 
> > On 08/11/23 13:36, Harry Wentland wrote:
> > > When the floor LUT index (drm_fixp2int(lut_index) is the last
> > > index of the array the ceil LUT index will point to an entry
> > > beyond the array. Make sure we guard against it and use the
> > > value of the floor LUT index.
> > > 
> > > v3:
> > >  - Drop bits from commit description that didn't contribute
> > >    anything of value
> > > 
> > > Signed-off-by: Harry Wentland <harry.wentl...@amd.com>
> > > Cc: Arthur Grillo <arthurgri...@riseup.net>
> > 
> > LGTM
> > Reviewed-by: Arthur Grillo <arthurgri...@riseup.net>
> 
> Nice catch. Thanks!
> 
> I'll already apply this one to drm-misc-next too.

It's a reminder to myself to add the missing fixes tag:

Fixes: db1f254f2cfaf ("drm/vkms: Add support to 1D gamma LUT")
Reviewed-by: Melissa Wen <m...@igalia.com>
> 
> CC'ing other VKMS maintainers/reviewers.
> 
> Melissa
> 
> > 
> > Best Regards,
> > ~Arthur Grillo
> > 
> > > ---
> > >  drivers/gpu/drm/vkms/vkms_composer.c | 14 ++++++++++----
> > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
> > > b/drivers/gpu/drm/vkms/vkms_composer.c
> > > index 6f942896036e..25b6b73bece8 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_composer.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> > > @@ -123,6 +123,8 @@ static u16 apply_lut_to_channel_value(const struct 
> > > vkms_color_lut *lut, u16 chan
> > >                                 enum lut_channel channel)
> > >  {
> > >   s64 lut_index = get_lut_index(lut, channel_value);
> > > + u16 *floor_lut_value, *ceil_lut_value;
> > > + u16 floor_channel_value, ceil_channel_value;
> > >  
> > >   /*
> > >    * This checks if `struct drm_color_lut` has any gap added by the 
> > > compiler
> > > @@ -130,11 +132,15 @@ static u16 apply_lut_to_channel_value(const struct 
> > > vkms_color_lut *lut, u16 chan
> > >    */
> > >   static_assert(sizeof(struct drm_color_lut) == sizeof(__u16) * 4);
> > >  
> > > - u16 *floor_lut_value = (__u16 *)&lut->base[drm_fixp2int(lut_index)];
> > > - u16 *ceil_lut_value = (__u16 *)&lut->base[drm_fixp2int_ceil(lut_index)];
> > > + floor_lut_value = (__u16 *)&lut->base[drm_fixp2int(lut_index)];
> > > + if (drm_fixp2int(lut_index) == (lut->lut_length - 1))
> > > +         /* We're at the end of the LUT array, use same value for ceil 
> > > and floor */
> > > +         ceil_lut_value = floor_lut_value;
> > > + else
> > > +         ceil_lut_value = (__u16 
> > > *)&lut->base[drm_fixp2int_ceil(lut_index)];
> > >  
> > > - u16 floor_channel_value = floor_lut_value[channel];
> > > - u16 ceil_channel_value = ceil_lut_value[channel];
> > > + floor_channel_value = floor_lut_value[channel];
> > > + ceil_channel_value = ceil_lut_value[channel];
> > >  
> > >   return lerp_u16(floor_channel_value, ceil_channel_value,
> > >                   lut_index & DRM_FIXED_DECIMAL_MASK);

Reply via email to