On Mon, Aug 31, 2020 at 01:38:58PM +0000, Sidong Yang wrote:
> On Mon, Aug 31, 2020 at 11:39:10AM +0000, Simon Ser wrote:
> > On Saturday, August 29, 2020 4:06 PM, Sidong Yang <realwa...@gmail.com>
> > wrote:
> >
> > > Currently vkms module doesn't support gamma function for userspace. so igt
> > > subtests in kms_plane(pixel-format-pipe-A-plan) failed for calling
> > > drmModeCrtcSetGamma().
> >
>
> Hi, Simon.
> Thanks for review.
>
> > It doesn't seem like this IGT test's goal is to exercise support for
> > gamma LUTs. Does the test just tries to reset the gamma LUT to linear?
> > If so, I think the IGT test should be fixed to ignore "I don't support
> > gamma" errors.
>
> It seems like that IGT test pixel-format is to make gamma lut like below.
>
> for (i = 0; i < lut_size; i++)
> lut[i] = (i * 0xffff / (lut_size - 1)) & mask;
>
> And set this table to drm driver. and test begins. It's the test about pixel
> format. I think you're right. It's not about gamma lut.
The point of the gamma LUT stuff in the pixel format test is to throw
away a bunch of the lsbs so that the test passes when the result is
"close enough" to the 8bpc RGB reference image. Without it we would
never get a crc match when testing non-8bpc or YCbCr formats.
> >
> > > This patch set gamma_set interface in vkms_crtc_funcs for
> > > support gamma function. With initializing crtc, added calls for setting
> > > gamma
> > > size. it pass the test after this patch.
> > >
> > > Cc: Daniel Vetter<dan...@ffwll.ch>
> > > Cc: Rodrigo Siqueira <rodrigosiqueiram...@gmail.com>
> > > Cc: Haneen Mohammed <hamohammed...@gmail.com>
> > >
> > > Signed-off-by: Sidong Yang <realwa...@gmail.com>
> > > ---
> > > drivers/gpu/drm/vkms/vkms_crtc.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > index ac85e17428f8..643435fb2ee6 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > @@ -160,6 +160,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > > .get_crc_sources = vkms_get_crc_sources,
> > > .set_crc_source = vkms_set_crc_source,
> > > .verify_crc_source = vkms_verify_crc_source,
> > > + .gamma_set = drm_atomic_helper_legacy_gamma_set,
> >
> > Why does VKMS need to use a legacy helper?
>
> drm_crtc_enable_color_mgmt() enables properties about gamma/degamma lut. And
> legacy helper just saves lut data from userspace to drm property blob. It
> seems
> that it's convenient way to implement .gamma_set.
>
> > It seems like this patch just advertises support for gamma LUTs, but
> > ignores any value set by user-space. If VKMS advertises support for
> > gamma LUTs, it needs to take the LUT into account when blending planes.
>
> Yes, This patch doesn't use gamma lut passed by user. lut should be used for
> calculating pixel value. For vkms, Maybe lut will be used in making crc value?
> If so, I'll try to write next patch for it.
>
> Thanks,
> -Sidong
>
> >
> > > };
> > >
> > > static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
> > > @@ -275,6 +276,13 @@ int vkms_crtc_init(struct drm_device *dev, struct
> > > drm_crtc *crtc,
> > > return ret;
> > > }
> > >
> > > + ret = drm_mode_crtc_set_gamma_size(crtc, 256);
> > > + if (ret) {
> > > + DRM_ERROR("Failed to set gamma size\n");
> > > + return ret;
> > > + }
> > > + drm_crtc_enable_color_mgmt(crtc, 0, false, 256);
> > > +
> > > drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
> > >
> > > spin_lock_init(&vkms_out->lock);
> > > --
> > > 2.17.1
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
