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.wentland@xxxxxxx> > > > Cc: Arthur Grillo <arthurgrillo@xxxxxxxxxx> > > > > LGTM > > Reviewed-by: Arthur Grillo <arthurgrillo@xxxxxxxxxx> > > 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 <mwen@xxxxxxxxxx> > > 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);