From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH v2 1/3] drm/vc4: Expose gamma as atomic property Date: Fri, 30 Mar 2018 09:05:32 -0700 Message-ID: <87fu4hwnub.fsf@anholt.net> References: <20180325015240.75464-1-stschake@gmail.com> <20180325015240.75464-3-stschake@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1627697841==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id D5BEF6E0C8 for ; Fri, 30 Mar 2018 16:05:36 +0000 (UTC) In-Reply-To: <20180325015240.75464-3-stschake@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Cc: airlied@linux.ie, linux-rpi-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org, Stefan Schake List-Id: dri-devel@lists.freedesktop.org --===============1627697841== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Stefan Schake writes: > We are an atomic driver so the gamma LUT should also be exposed as a > CRTC property through the DRM atomic color management. This will also > take care of the legacy path for us. > > Signed-off-by: Stefan Schake > --- > v2: Use drm_color_lut_size for LUT length > > drivers/gpu/drm/vc4/vc4_crtc.c | 24 +++++++++++++----------- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crt= c.c > index bf4667481935..239215cb3274 100644 > --- a/drivers/gpu/drm/vc4/vc4_crtc.c > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c > @@ -298,23 +298,21 @@ vc4_crtc_lut_load(struct drm_crtc *crtc) > HVS_WRITE(SCALER_GAMDATA, vc4_crtc->lut_b[i]); > } >=20=20 > -static int > -vc4_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, > - uint32_t size, > - struct drm_modeset_acquire_ctx *ctx) > +static void > +vc4_crtc_update_gamma_lut(struct drm_crtc *crtc) > { > struct vc4_crtc *vc4_crtc =3D to_vc4_crtc(crtc); > + struct drm_color_lut *lut =3D crtc->state->gamma_lut->data; > + u32 length =3D drm_color_lut_size(crtc->state->gamma_lut); > u32 i; >=20=20 > - for (i =3D 0; i < size; i++) { > - vc4_crtc->lut_r[i] =3D r[i] >> 8; > - vc4_crtc->lut_g[i] =3D g[i] >> 8; > - vc4_crtc->lut_b[i] =3D b[i] >> 8; > + for (i =3D 0; i < length; i++) { > + vc4_crtc->lut_r[i] =3D drm_color_lut_extract(lut[i].red, 8); > + vc4_crtc->lut_g[i] =3D drm_color_lut_extract(lut[i].green, 8); > + vc4_crtc->lut_b[i] =3D drm_color_lut_extract(lut[i].blue, 8); > } >=20=20 > vc4_crtc_lut_load(crtc); > - > - return 0; > } >=20=20 > static u32 vc4_get_fifo_full_level(u32 format) > @@ -699,6 +697,9 @@ static void vc4_crtc_atomic_flush(struct drm_crtc *cr= tc, > if (crtc->state->active && old_state->active) > vc4_crtc_update_dlist(crtc); >=20=20 > + if (crtc->state->color_mgmt_changed && crtc->state->gamma_lut) > + vc4_crtc_update_gamma_lut(crtc); Don't we need to set things back to linear if gamma_lut is NULL? (maybe by updating the SCALER_DISPBKGND_GAMMA flag on the HVS channel)=20 Other than that, this looks great. > + > if (debug_dump_regs) { > DRM_INFO("CRTC %d HVS after:\n", drm_crtc_index(crtc)); > vc4_hvs_dump_state(dev); > @@ -909,7 +910,7 @@ static const struct drm_crtc_funcs vc4_crtc_funcs =3D= { > .reset =3D vc4_crtc_reset, > .atomic_duplicate_state =3D vc4_crtc_duplicate_state, > .atomic_destroy_state =3D vc4_crtc_destroy_state, > - .gamma_set =3D vc4_crtc_gamma_set, > + .gamma_set =3D drm_atomic_helper_legacy_gamma_set, > .enable_vblank =3D vc4_enable_vblank, > .disable_vblank =3D vc4_disable_vblank, > }; > @@ -1035,6 +1036,7 @@ static int vc4_crtc_bind(struct device *dev, struct= device *master, void *data) > primary_plane->crtc =3D crtc; > vc4_crtc->channel =3D vc4_crtc->data->hvs_channel; > drm_mode_crtc_set_gamma_size(crtc, ARRAY_SIZE(vc4_crtc->lut_r)); > + drm_crtc_enable_color_mgmt(crtc, 0, false, crtc->gamma_size); >=20=20 > /* Set up some arbitrary number of planes. We're not limited > * by a set number of physical registers, just the space in > --=20 > 2.14.1 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlq+YEwACgkQtdYpNtH8 nugWMA/9HxAl35YtxA1VKKddboFRYohSM9QMoFBhuz53oJzHnaeD3uJ8ti2p+lT3 UpHSKtS2viGt09J6kmv0TNPRlZyk+dLEpaX9bP7+o7GjooYLACOWzjHMkJ8DqrHh Yeuc6n9Vc5re1iGkKPrtI7XJYguzOW+FixY6Qg9vatBQgU9aw40aqdITIIm8klCx b/q5Ojts6Jy2vu5qjo9c31gPSWu1bya81trYnHdOHDypRSld23/eSUi/ip2Be9E6 W8t+7oTeF1qL2oLYSqCqcDZ9CCAVPhxUWs1O89bDdu6nG1M9vygTN24tefh7acq+ 4nd7WwLL433H1tsX6BoWbt9hGfR3Xc1Fqct3pm0jfjsq9wJ34ZH9XmCnjSky4cq3 jM01deT6jR6cjZhfmVqMMMqUQjA190lcwhrXIJvnAEZbkZRY9dK5uPM2NZ/ni7uL DoEenJnB5eu1KqvKIh35agInw0gzwhI9lfrg5QaAs8KsE2/8Dn/USVm+kUoPAUd/ 8LxHHBtsjqW+eqwn1l573FiFyaEAMzfY3OTEcxX+Vg29zgDbrEw1AmpAnXj0T4Rs l0J1hJ7rDWZj/e+g9lL00Zt4rmOSO6zKNvChSpmw/+z7RakyOipgmUWoeGMhaGS+ u/eXOsSnGzsBpHGsZah6MpDQxDVfTRqvwgLMN0We+8NlXJRdLsQ= =iniP -----END PGP SIGNATURE----- --=-=-=-- --===============1627697841== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1627697841==--