Hi Tomi,
Thank you for the patch.
On Tue, Dec 08, 2020 at 03:57:58PM +0200, Tomi Valkeinen wrote:
We currently have drm_atomic_helper_legacy_gamma_set() helper which can be used to handle legacy gamma-set ioctl. drm_atomic_helper_legacy_gamma_set() sets GAMMA_LUT, and clears CTM and DEGAMMA_LUT. This works fine on HW where we have either:
degamma -> ctm -> gamma -> out
or
ctm -> gamma -> out
However, if the HW has gamma table before ctm, the atomic property should be DEGAMMA_LUT, and thus we have:
degamma -> ctm -> out
This is fine for userspace which sets gamma table using the properties, as the userspace can check for the existence of gamma & degamma, but the legacy gamma-set ioctl does not work.
This patch fixes the issue by changing drm_atomic_helper_legacy_gamma_set() so that GAMMA_LUT will be used if it exists, and DEGAMMA_LUT will be used as a fallback.
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/gpu/drm/drm_atomic_helper.c | 15 ++++++++++++--- drivers/gpu/drm/drm_color_mgmt.c | 4 ++++ drivers/gpu/drm/drm_fb_helper.c | 8 ++++++-- include/drm/drm_crtc.h | 3 +++ 4 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ba1507036f26..117b186fe646 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3512,6 +3512,10 @@ EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
- that support color management through the DEGAMMA_LUT/GAMMA_LUT
- properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
- how the atomic color management and gamma tables work.
- This function uses the GAMMA_LUT or DEGAMMA_LUT property for the gamma table.
- GAMMA_LUT property is used if it exists, and DEGAMMA_LUT property is used as
*/
- a fallback.
int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, @@ -3526,6 +3530,9 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, int i, ret = 0; bool replaced;
- if (!crtc->has_gamma_prop && !crtc->has_degamma_prop)
return -ENODEV;- state = drm_atomic_state_alloc(crtc->dev); if (!state) return -ENOMEM;
@@ -3554,10 +3561,12 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, goto fail; }
- /* Reset DEGAMMA_LUT and CTM properties. */
- replaced = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
- /* Set GAMMA/DEGAMMA_LUT and reset DEGAMMA/GAMMA_LUT and CTM */
- replaced = drm_property_replace_blob(&crtc_state->degamma_lut,
crtc->has_gamma_prop ? NULL : blob);
- replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
crtc->has_gamma_prop ? blob : NULL);crtc_state->color_mgmt_changed |= replaced;
ret = drm_atomic_commit(state);
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index 3bcabc2f6e0e..956e59d5f6a7 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -176,6 +176,8 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, degamma_lut_size); }
- crtc->has_degamma_prop = !!degamma_lut_size;
- if (has_ctm) drm_object_attach_property(&crtc->base, config->ctm_property, 0);
@@ -187,6 +189,8 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, config->gamma_lut_size_property, gamma_lut_size); }
- crtc->has_gamma_prop = !!gamma_lut_size;
} EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 25edf670867c..b0906ef97617 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1001,6 +1001,9 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info) drm_client_for_each_modeset(modeset, &fb_helper->client) { crtc = modeset->crtc;
if (!crtc->has_gamma_prop && !crtc->has_degamma_prop)continue;- if (!gamma_lut) gamma_lut = setcmap_new_gamma_lut(crtc, cmap); if (IS_ERR(gamma_lut)) {
@@ -1015,11 +1018,12 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info) goto out_state; }
/* Set GAMMA/DEGAMMA_LUT and reset DEGAMMA/GAMMA_LUT and CTM */
NULL);
crtc->has_gamma_prop ? NULL : gamma_lut);
gamma_lut);
crtc->has_gamma_prop ? gamma_lut : NULL);diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index ba839e5e357d..4d9e217e5040 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1084,6 +1084,9 @@ struct drm_crtc { */ uint16_t *gamma_store;
- bool has_gamma_prop : 1;
- bool has_degamma_prop : 1;
- /** @helper_private: mid-layer private data */ const struct drm_crtc_helper_funcs *helper_private;