[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8063b9555d5bce6f2c002e49da3b7afaca9ae0b4.camel@collabora.com>
Date: Mon, 30 Sep 2019 17:53:00 -0300
From: Ezequiel Garcia <ezequiel@...labora.com>
To: Jacopo Mondi <jacopo+renesas@...ndi.org>,
laurent.pinchart@...asonboard.com,
kieran.bingham+renesas@...asonboard.com, geert@...ux-m68k.org,
horms@...ge.net.au, uli+renesas@...nd.eu,
VenkataRajesh.Kalakodima@...bosch.com, airlied@...ux.ie,
daniel@...ll.ch
Cc: muroya@....co.jp, koji.matsuoka.xm@...esas.com,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-renesas-soc@...r.kernel.org,
Harsha.ManjulaMallikarjun@...bosch.com,
Doug Anderson <dianders@...gle.com>,
Heiko Stuebner <heiko@...ech.de>
Subject: Re: [PATCH v4 8/9] drm: rcar-du: kms: Update CMM in atomic commit
tail
+Doug, Heiko:
On Fri, 2019-09-06 at 15:54 +0200, Jacopo Mondi wrote:
> Update CMM settings at in the atomic commit tail helper method.
> The CMM is updated with new gamma values provided to the driver
> in the GAMMA_LUT blob property.
>
> When resuming from system suspend, the DU driver is responsible for
> reprogramming and enabling the CMM unit if it was in use at the time the
> system entered the suspend state. Force the color_mgmt_changed flag to
> true if the DRM gamma lut color transformation property was set in the
> CRTC state duplicated at suspend time, as the CMM gets reprogrammed only
> if said flag is active in the rcar_du_atomic_commit_update_cmm() method.
>
> Reviewed-by: Ulrich Hecht <uli+renesas@...nd.eu>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@...ndi.org>
> ---
>
> Daniel could you have a look if resume bits are worth being moved to the
> DRM core? The color_mgmt_changed flag is set to false when the state is
> duplicated if I read the code correctly, but when this happens in a
> suspend/resume sequence its value should probably be restored to true if
> any color management property was set in the crtc state when system entered
> suspend.
>
Perhaps we can use the for_each_new_crtc_in_state() helper here,
and move it to the core like this:
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3234,8 +3234,20 @@ int drm_atomic_helper_resume(struct
drm_device *dev,
struct drm_atomic_state *state)
{
struct drm_modeset_acquire_ctx ctx;
+ struct drm_crtc_state
*crtc_state;
+ struct drm_crtc *crtc;
+ unsigned int i;
int err;
+ for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+
/*
+ * Force re-enablement of CMM after system resume if any
+ * of the DRM color transformation properties
was set in
+ * the state saved at system suspend time.
+ */
+ if (crtc_state->gamma_lut)
+
crtc_state->color_mgmt_changed = true;
+ }
This probably is wrong, and should be instead constrained to some
condition of some sort.
FWIW, the Rockchip DRM is going to need this as well.
Any ideas?
Thanks,
Ezequiel
Powered by blists - more mailing lists