From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
dri-devel@lists.freedesktop.org, Sekhar Nori <nsekhar@ti.com>,
Jyri Sarha <jsarha@ti.com>, Nikhil Devshatwar <nikhil.nd@ti.com>
Subject: Re: [PATCH v2 1/5] drm: add legacy support for using degamma for gamma
Date: Mon, 30 Nov 2020 12:38:40 +0200 [thread overview]
Message-ID: <20201130103840.GR4141@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20201103080310.164453-2-tomi.valkeinen@ti.com>
Hi Tomi,
Thank you for the patch.
On Tue, Nov 03, 2020 at 10:03:06AM +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 adds a new helper, drm_atomic_helper_legacy_degamma_set(),
> which can be used instead of drm_atomic_helper_legacy_gamma_set() when
> the DEGAMMA_LUT is the underlying property that needs to be set.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 81 ++++++++++++++++++++++-------
> include/drm/drm_atomic_helper.h | 4 ++
> 2 files changed, 65 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ddd0e3239150..23cbed541dc7 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3499,24 +3499,11 @@ int drm_atomic_helper_page_flip_target(struct drm_crtc *crtc,
> }
> EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
>
> -/**
> - * drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction table
> - * @crtc: CRTC object
> - * @red: red correction table
> - * @green: green correction table
> - * @blue: green correction table
> - * @size: size of the tables
> - * @ctx: lock acquire context
> - *
> - * Implements support for legacy gamma correction table for drivers
> - * 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.
> - */
> -int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> - u16 *red, u16 *green, u16 *blue,
> - uint32_t size,
> - struct drm_modeset_acquire_ctx *ctx)
> +static int legacy_gamma_degamma_set(struct drm_crtc *crtc,
> + u16 *red, u16 *green, u16 *blue,
> + uint32_t size,
> + struct drm_modeset_acquire_ctx *ctx,
> + bool use_degamma)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_atomic_state *state;
> @@ -3555,9 +3542,11 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> }
>
> /* Reset DEGAMMA_LUT and CTM properties. */
> - replaced = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
> + replaced = drm_property_replace_blob(&crtc_state->degamma_lut,
> + use_degamma ? blob : NULL);
> replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
> - replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
> + replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
> + use_degamma ? NULL : blob);
> crtc_state->color_mgmt_changed |= replaced;
>
> ret = drm_atomic_commit(state);
> @@ -3567,8 +3556,60 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> drm_property_blob_put(blob);
> return ret;
> }
> +
> +/**
> + * drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction table using gamma_lut
> + * @crtc: CRTC object
> + * @red: red correction table
> + * @green: green correction table
> + * @blue: green correction table
> + * @size: size of the tables
> + * @ctx: lock acquire context
> + *
> + * Implements support for legacy gamma correction table for drivers
> + * 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 GAMMA_LUT property for the gamma table. This function
s/uses/uses the/
s/This function$/It/
Same below.
> + * can be used when the driver exposes either only GAMMA_LUT or both GAMMA_LUT
> + * and DEGAMMA_LUT.
> + */
> +int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> + u16 *red, u16 *green, u16 *blue,
> + uint32_t size,
> + struct drm_modeset_acquire_ctx *ctx)
> +{
> + return legacy_gamma_degamma_set(crtc, red, green, blue, size, ctx, false);
> +}
I wonder, would it make sense to make this automatic by setting the
degamma LUT when only the DEGAMMA_LUT property exists, and the gamma LUT
otherwise ? Are there use cases for drm_atomic_helper_legacy_degamma_set
for drivers that support both gamma and degamma ?
> EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set);
>
> +/**
> + * drm_atomic_helper_legacy_degamma_set - set the legacy gamma correction table using degamma_lut
> + * @crtc: CRTC object
> + * @red: red correction table
> + * @green: green correction table
> + * @blue: green correction table
> + * @size: size of the tables
> + * @ctx: lock acquire context
> + *
> + * Implements support for legacy gamma correction table for drivers
> + * 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 DEGAMMA_LUT property for the gamma table. This function
> + * can be used when the driver exposes only DEGAMNMA_LUT.
> + */
> +int drm_atomic_helper_legacy_degamma_set(struct drm_crtc *crtc,
> + u16 *red, u16 *green, u16 *blue,
> + uint32_t size,
> + struct drm_modeset_acquire_ctx *ctx)
> +{
> + return legacy_gamma_degamma_set(crtc, red, green, blue, size, ctx, true);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_legacy_degamma_set);
> +
> /**
> * drm_atomic_helper_bridge_propagate_bus_fmt() - Propagate output format to
> * the input end of a bridge
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 85df04c8e62f..561c78680388 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -151,6 +151,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> u16 *red, u16 *green, u16 *blue,
> uint32_t size,
> struct drm_modeset_acquire_ctx *ctx);
> +int drm_atomic_helper_legacy_degamma_set(struct drm_crtc *crtc,
> + u16 *red, u16 *green, u16 *blue,
> + uint32_t size,
> + struct drm_modeset_acquire_ctx *ctx);
>
> /**
> * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-11-30 10:38 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-03 8:03 [PATCH v2 0/5] drm/omap: add color mgmt support Tomi Valkeinen
2020-11-03 8:03 ` [PATCH v2 1/5] drm: add legacy support for using degamma for gamma Tomi Valkeinen
2020-11-30 10:38 ` Laurent Pinchart [this message]
2020-11-30 12:12 ` Tomi Valkeinen
2020-11-30 14:10 ` Daniel Vetter
2020-12-02 11:52 ` Tomi Valkeinen
2020-12-02 12:38 ` Daniel Vetter
2020-12-03 12:31 ` Ville Syrjälä
2020-12-03 12:35 ` Tomi Valkeinen
2020-11-03 8:03 ` [PATCH v2 2/5] drm/omap: use degamma property for gamma table Tomi Valkeinen
2020-11-30 10:39 ` Laurent Pinchart
2020-11-03 8:03 ` [PATCH v2 3/5] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix Tomi Valkeinen
2020-11-30 10:41 ` Laurent Pinchart
2020-11-30 11:39 ` Tomi Valkeinen
2020-11-03 8:03 ` [PATCH v2 4/5] drm/omap: rearrange includes in omapdss.h Tomi Valkeinen
2020-11-30 10:42 ` Laurent Pinchart
2020-11-03 8:03 ` [PATCH v2 5/5] drm/omap: Enable COLOR_ENCODING and COLOR_RANGE properties for planes Tomi Valkeinen
2020-11-30 10:50 ` Laurent Pinchart
2020-11-30 11:53 ` Tomi Valkeinen
2020-11-30 14:19 ` Laurent Pinchart
2020-11-30 14:36 ` Tomi Valkeinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201130103840.GR4141@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=jsarha@ti.com \
--cc=nikhil.nd@ti.com \
--cc=nsekhar@ti.com \
--cc=pekka.paalanen@collabora.com \
--cc=tomi.valkeinen@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).