From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jyri Sarha Subject: Re: [PATCH v5 1/4] drm: drm_helper_crtc_enable_color_mgmt() => drm_crtc_enable_color_mgmt() Date: Thu, 26 May 2016 12:59:09 +0300 Message-ID: <5746C8ED.4050208@ti.com> References: <0816cb2f539f2a8c7cd0d2e1252e6902f687e1ae.1464251087.git.jsarha@ti.com> <5746BC5C.1070600@ti.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0053015743==" Return-path: Received: from arroyo.ext.ti.com (arroyo.ext.ti.com [198.47.19.12]) by gabe.freedesktop.org (Postfix) with ESMTPS id 309636EA77 for ; Thu, 26 May 2016 09:59:24 +0000 (UTC) In-Reply-To: <5746BC5C.1070600@ti.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Tomi Valkeinen , dri-devel@lists.freedesktop.org, daniel@ffwll.ch Cc: peter.ujfalusi@ti.com, laurent.pinchart@ideasonboard.com List-Id: dri-devel@lists.freedesktop.org --===============0053015743== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="hUce59HOGvRIpA7dG6864VoS8o41gctol" --hUce59HOGvRIpA7dG6864VoS8o41gctol Content-Type: multipart/mixed; boundary="hreN241958GbILAQ1IAArvweFeSMGbv7o" From: Jyri Sarha To: Tomi Valkeinen , dri-devel@lists.freedesktop.org, daniel@ffwll.ch Cc: airlied@linux.ie, peter.ujfalusi@ti.com, bparrot@ti.com, laurent.pinchart@ideasonboard.com, lionel.g.landwerlin@intel.com Message-ID: <5746C8ED.4050208@ti.com> Subject: Re: [PATCH v5 1/4] drm: drm_helper_crtc_enable_color_mgmt() => drm_crtc_enable_color_mgmt() References: <0816cb2f539f2a8c7cd0d2e1252e6902f687e1ae.1464251087.git.jsarha@ti.com> <5746BC5C.1070600@ti.com> In-Reply-To: <5746BC5C.1070600@ti.com> --hreN241958GbILAQ1IAArvweFeSMGbv7o Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/26/16 12:05, Tomi Valkeinen wrote: > Hi Jyri, Daniel, >=20 > On 26/05/16 11:35, Jyri Sarha wrote: >> Add drm_crtc_enable_color_mgmt(), remove drm_helper_crtc_enable_color_= mgmt() >> and update drm/i915-driver (the only user of the old function). >> >> The new function is more flexible. It allows driver to enable only the= >> features it has without forcing to enable all three color management >> properties: degamma lut, csc matrix (ctm), and gamma lut. >> >> Suggested-by: Daniel Vetter >> Signed-off-by: Jyri Sarha >=20 >> +void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, >> + uint degamma_lut_size, >> + bool has_ctm, >> + uint gamma_lut_size) >> +{ >> + struct drm_device *dev =3D crtc->dev; >> + struct drm_mode_config *config =3D &dev->mode_config; >> + >> + if (degamma_lut_size) { >> + drm_object_attach_property(&crtc->base, >> + config->degamma_lut_property, 0); >> + drm_object_attach_property(&crtc->base, >> + config->degamma_lut_size_property, >> + degamma_lut_size); >> + } >> + >> + if (has_ctm) >> + drm_object_attach_property(&crtc->base, >> + config->ctm_property, 0); >> + >> + if (gamma_lut_size) { >> + drm_object_attach_property(&crtc->base, >> + config->gamma_lut_property, 0); >> + drm_object_attach_property(&crtc->base, >> + config->gamma_lut_size_property, >> + gamma_lut_size); >> + } >> +} >=20 > As I mentioned, I think it would make sense to call > drm_mode_crtc_set_gamma_size() here. At least from omapdrm perspective.= >=20 > I had a look at i915, and that one looks a bit odd. It always sets > drm_mode_crtc_set_gamma_size(256), but then only calls > drm_helper_crtc_enable_color_mgmt() if > INTEL_INFO(dev)->color.gamma_lut_size > 0, and gives gamma_lut_size as > the size. >=20 > Is there some legacy stuff at play here? drm_mode_crtc_set_gamma_size()= > should always be 256 (as X expects that), but the GAMMA_LUT property ca= n > give the real gamma lut size? >=20 This indeed seems to be the case. If call drm_mode_crtc_set_gamma_size, with 256, but set the gamma_lut_size_property to 1024, the X still works but trough atomic API I can use the full length gamma table. I wonder if should do yet one more patch-round? BR, Jyri --hreN241958GbILAQ1IAArvweFeSMGbv7o-- --hUce59HOGvRIpA7dG6864VoS8o41gctol Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJXRsjzAAoJEJA2s1DX1hlBppoQAN7ftlFRlCR+/PKLVJmwhKSY QtI4gnh3q3gCIuAabKUczqxYG85AuxVssh1ZpPGXaaW4YpjkfJJK30K6HpC0bJSA 1TDhOmUuCwAs7W9zYIleoBQY2nAW8qW7Qra1AkRvP9j20wj8O0Ws6chNhLeBOcDt vywhI2f2Y1LS0OeWQ2GjycLzJawIcuYT7H69+4pUsFIYKdxyNMbN23oMl0Nlpdpz 2k9mQoZhzS3xTZA16sBpsbkUXHue9DVhOl+21nC9kBADf/8IbL2+oyW4Q/Tn7tax j8d2DHbSkRbcF5K8AYj8unqWrLbL7HZDYujLDbfS7/4YahEOMupllTnXHNKO1tvV q6P5GGJwt93jJA09CP+8ddFxO1VOJUgO19H1apy2FtUfhSSHbgHdBLaVqwv7D4ui RvzCytjk39Mea/XNIAkXwRffdyd6wxaKXa7TjWp2qbA6CN6lqtqAfBgOJEUkixMS oDxP6jw7pyrpo8/sQZMTMQCOl+Y6YncShhwneyps5wgEGSwZReqCGLHMi0MMn/3M YsDaCZc4NBd0PGUYK0Rmz9pAdlXn7fey2KktYnuHq+NN51r3U03VS2N810qQFxV0 YVYHc36QODo2g0V73HmJchYlfsOHc4cxNap5XYDFjJo7fQBqm+i2qpHjDeIV+Dah /JP+jxAyafvoEDuT0njd =h8H3 -----END PGP SIGNATURE----- --hUce59HOGvRIpA7dG6864VoS8o41gctol-- --===============0053015743== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0053015743==--