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 realwakka@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 Vetterdaniel@ffwll.ch Cc: Rodrigo Siqueira rodrigosiqueiramelo@gmail.com Cc: Haneen Mohammed hamohammed.sa@gmail.com
Signed-off-by: Sidong Yang realwakka@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