From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9144C4361B for ; Mon, 7 Dec 2020 15:32:01 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5DED023730 for ; Mon, 7 Dec 2020 15:32:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5DED023730 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3D6166E584; Mon, 7 Dec 2020 15:32:00 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 89A336E584 for ; Mon, 7 Dec 2020 15:31:58 +0000 (UTC) IronPort-SDR: 9x6x8ZRvf0lbWcvNxeq318xOVzlhlZJgoHosmEg6MaduSwZQqxnkT5qCT4VqzGZYTK/ZeiB0vL O5i7hR1RVU7w== X-IronPort-AV: E=McAfee;i="6000,8403,9827"; a="258428435" X-IronPort-AV: E=Sophos;i="5.78,400,1599548400"; d="scan'208";a="258428435" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Dec 2020 07:31:57 -0800 IronPort-SDR: PUAbyOwzfzuAJ9/aLW7yMe9mfSTVMrs/J7+FCT341QOq0VpGkTx6EwUqoxgLqQ62VucELBJkhF AhU27k6yHWMQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.78,400,1599548400"; d="scan'208";a="317216558" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by fmsmga008.fm.intel.com with SMTP; 07 Dec 2020 07:31:52 -0800 Received: by stinkbox (sSMTP sendmail emulation); Mon, 07 Dec 2020 17:31:52 +0200 Date: Mon, 7 Dec 2020 17:31:52 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Laurent Pinchart Subject: Re: [PATCH 1/2] drm: add legacy support for using degamma for gamma Message-ID: References: <20201203114845.232911-1-tomi.valkeinen@ti.com> <20201203114845.232911-2-tomi.valkeinen@ti.com> <20201204223525.GJ4109@pendragon.ideasonboard.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201204223525.GJ4109@pendragon.ideasonboard.com> X-Patchwork-Hint: comment X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paul Cercueil , Alexandre Torgue , Sandy Huang , David Airlie , Philippe Cornu , dri-devel@lists.freedesktop.org, Russell King , Yannick Fertre , Tomi Valkeinen , Thomas Zimmermann , Matthias Brugger , Vincent Abriou , Maxime Coquelin Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Sat, Dec 05, 2020 at 12:35:25AM +0200, Laurent Pinchart wrote: > Hi Tomi, > = > Thank you for the patch. > = > On Thu, Dec 03, 2020 at 01:48:44PM +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 > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 18 +++++++++++++++--- > > drivers/gpu/drm/drm_color_mgmt.c | 4 ++++ > > include/drm/drm_crtc.h | 3 +++ > > 3 files changed, 22 insertions(+), 3 deletions(-) > > = > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_= atomic_helper.c > > index ba1507036f26..fe59c8ea42a9 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 cha= pter for > > * how the atomic color management and gamma tables work. > > + * > > + * This function uses the GAMMA_LUT or DEGAMMA_LUT property for the ga= mma table. > > + * GAMMA_LUT property is used if it exists, and DEGAMMA_LUT property i= s used as > > + * a fallback. > > */ > > int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc, > > u16 *red, u16 *green, u16 *blue, > > @@ -3525,6 +3529,12 @@ int drm_atomic_helper_legacy_gamma_set(struct dr= m_crtc *crtc, > > struct drm_color_lut *blob_data; > > int i, ret =3D 0; > > bool replaced; > > + bool use_degamma; > > + > > + if (!crtc->has_gamma_prop && !crtc->has_degamma_prop) > > + return -ENODEV; > > + > > + use_degamma =3D !crtc->has_gamma_prop; > > = > > state =3D drm_atomic_state_alloc(crtc->dev); > > if (!state) > > @@ -3554,10 +3564,12 @@ int drm_atomic_helper_legacy_gamma_set(struct d= rm_crtc *crtc, > > goto fail; > > } > > = > > - /* Reset DEGAMMA_LUT and CTM properties. */ > > - replaced =3D drm_property_replace_blob(&crtc_state->degamma_lut, NUL= L); > > + /* Set GAMMA/DEGAMMA_LUT and reset DEGAMMA/GAMMA_LUT and CTM */ > > + replaced =3D drm_property_replace_blob(&crtc_state->degamma_lut, > > + use_degamma ? blob : NULL); > > replaced |=3D drm_property_replace_blob(&crtc_state->ctm, NULL); > > - replaced |=3D drm_property_replace_blob(&crtc_state->gamma_lut, blob); > > + replaced |=3D drm_property_replace_blob(&crtc_state->gamma_lut, > > + use_degamma ? NULL : blob); > > crtc_state->color_mgmt_changed |=3D replaced; > > = > > ret =3D drm_atomic_commit(state); > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_col= or_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 *cr= tc, > > degamma_lut_size); > > } > > = > > + crtc->has_degamma_prop =3D !!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 *cr= tc, > > config->gamma_lut_size_property, > > gamma_lut_size); > > } > > + > > + crtc->has_gamma_prop =3D !!gamma_lut_size; > > } > > EXPORT_SYMBOL(drm_crtc_enable_color_mgmt); > > = > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > index ba839e5e357d..9e1f06047e3d 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; > > + bool has_degamma_prop; > = > We could use a bitfield to save a bit of memory. Apart from that, the > patch looks good to me. Or we could just check if the crtc has the relevant prop or not. > = > Reviewed-by: Laurent Pinchart > = > > + > > /** @helper_private: mid-layer private data */ > > const struct drm_crtc_helper_funcs *helper_private; > > = > = > -- = > Regards, > = > Laurent Pinchart > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- = Ville Syrj=E4l=E4 Intel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel