lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 31 Aug 2020 13:38:58 +0000 From: Sidong Yang <realwakka@...il.com> To: Simon Ser <contact@...rsion.fr> Cc: Daniel Vetter <daniel@...ll.ch>, Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>, Haneen Mohammed <hamohammed.sa@...il.com>, Emil Velikov <emil.l.velikov@...il.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>, Melissa Wen <melissa.srw@...il.com> Subject: Re: [PATCH] drm/vkms: add support for gamma_set interface On Mon, Aug 31, 2020 at 11:39:10AM +0000, Simon Ser wrote: > On Saturday, August 29, 2020 4:06 PM, Sidong Yang <realwakka@...il.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. > > > 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<daniel@...ll.ch> > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com> > > Cc: Haneen Mohammed <hamohammed.sa@...il.com> > > > > Signed-off-by: Sidong Yang <realwakka@...il.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 >
Powered by blists - more mailing lists