Re: [PATCH 1/9] drm: Add gamma mode property
On Wed, 2 Jun 2021 20:18:19 + "Shankar, Uma" wrote: > > -Original Message- > > From: Pekka Paalanen > > Sent: Wednesday, June 2, 2021 2:40 PM > > To: Shankar, Uma > > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, > > Bhanuprakash > > Subject: Re: [PATCH 1/9] drm: Add gamma mode property > > > > On Tue, 1 Jun 2021 16:11:27 +0530 > > Uma Shankar wrote: > > > > > Add a gamma mode property to enable various kind of gamma modes > > > supported by platforms like: Interpolated, Split, Multi Segmented, > > > Logarithmic etc. Userspace can get this property and should be able to > > > get the platform capabilities wrt various gamma modes possible and the > > > possible ranges. > > > > > > It can select one of the modes exposed as blob_id as an enum and set > > > the respective mode. > > > > > > It can then create the LUT and send it to driver using already > > > available GAMMA_LUT property as blob. > > > > > > Note: This is based on design by Ville and is being carried forward > > > based on his original idea. > > > > > > Signed-off-by: Uma Shankar > > > --- > > > drivers/gpu/drm/drm_atomic_uapi.c | 5 +++ > > > drivers/gpu/drm/drm_color_mgmt.c | 75 +++ > > > include/drm/drm_color_mgmt.h | 8 > > > include/drm/drm_crtc.h| 14 ++ > > > include/uapi/drm/drm_mode.h | 43 ++ > > > 5 files changed, 145 insertions(+) ... > > Hi, > > > > where is the UAPI documentation for this new GAMMA_MODE? > > As a userspace dev, I have no idea what to do with the above based on what's > > written here. > > Got that, I will add more details on the UAPI usage to make things a bit > clearer. > > > Also, reading the description of DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES in > > patch 5/9, what difference does it make whether userspace sets or does not > > set that > > cap? I don't understand the implications from the description. > > The reason we have this Client caps is to have it co-exist with legacy crtc > color properties. > The idea is that driver will describe the h/w luts to userspace through > GAMMA_MODE UAPI, > but the actual lut samples will still be sent through the legacy GAMMA_LUT > UAPI. This client > cap will help distinguish between legacy and this new implementation. > > I will add more details in the UAPI description to avoid ambiguity and > explain the rationale and > usage of this UAPI. > > Thanks Pekka for the looking into the series and the initial feedback. Thanks a lot, will be interesting to read those docs. pq pgpX40QqOpQsH.pgp Description: OpenPGP digital signature
RE: [PATCH 1/9] drm: Add gamma mode property
> -Original Message- > From: Pekka Paalanen > Sent: Wednesday, June 2, 2021 2:40 PM > To: Shankar, Uma > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Modem, > Bhanuprakash > Subject: Re: [PATCH 1/9] drm: Add gamma mode property > > On Tue, 1 Jun 2021 16:11:27 +0530 > Uma Shankar wrote: > > > Add a gamma mode property to enable various kind of gamma modes > > supported by platforms like: Interpolated, Split, Multi Segmented, > > Logarithmic etc. Userspace can get this property and should be able to > > get the platform capabilities wrt various gamma modes possible and the > > possible ranges. > > > > It can select one of the modes exposed as blob_id as an enum and set > > the respective mode. > > > > It can then create the LUT and send it to driver using already > > available GAMMA_LUT property as blob. > > > > Note: This is based on design by Ville and is being carried forward > > based on his original idea. > > > > Signed-off-by: Uma Shankar > > --- > > drivers/gpu/drm/drm_atomic_uapi.c | 5 +++ > > drivers/gpu/drm/drm_color_mgmt.c | 75 +++ > > include/drm/drm_color_mgmt.h | 8 > > include/drm/drm_crtc.h| 14 ++ > > include/uapi/drm/drm_mode.h | 43 ++ > > 5 files changed, 145 insertions(+) > > > > ... > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index > > 13eeba2a750a..b1eead03ebe8 100644 > > --- a/include/drm/drm_crtc.h > > +++ b/include/drm/drm_crtc.h > > @@ -262,6 +262,13 @@ struct drm_crtc_state { > > */ > > struct drm_property_blob *mode_blob; > > > > + /** > > +* @gamma_mode: This is a blob_id and exposes the platform capabilities > > +* wrt to various gamma modes and the respective lut ranges. This also > > +* helps user select a gamma mode amongst the supported ones. > > +*/ > > + u32 gamma_mode; > > + > > /** > > * @degamma_lut: > > * > > @@ -1096,6 +1103,13 @@ struct drm_crtc { > > */ > > struct drm_property *scaling_filter_property; > > > > + /** > > +* @gamma_mode_property: Optional CRTC property to enumerate and > > +* select the mode of the crtc gamma/degmama LUTs. This also exposes > > +* the lut ranges of the various supported gamma modes to userspace. > > +*/ > > + struct drm_property *gamma_mode_property; > > + > > /** > > * @state: > > * > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > > index 9b6722d45f36..d7758d351936 100644 > > --- a/include/uapi/drm/drm_mode.h > > +++ b/include/uapi/drm/drm_mode.h > > @@ -819,6 +819,49 @@ struct hdr_output_metadata { > > }; > > }; > > > > +/* > > + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means > the LUT > > + * can be used for either purpose, but not simultaneously. To expose > > + * modes that support gamma and degamma simultaneously the gamma mode > > + * must declare distinct DRM_MODE_LUT_GAMMA and > DRM_MODE_LUT_DEGAMMA > > + * ranges. > > + */ > > +/* LUT is for gamma (after CTM) */ > > +#define DRM_MODE_LUT_GAMMA BIT(0) > > +/* LUT is for degamma (before CTM) */ #define DRM_MODE_LUT_DEGAMMA > > +BIT(1) > > +/* linearly interpolate between the points */ #define > > +DRM_MODE_LUT_INTERPOLATE BIT(2) > > +/* > > + * the last value of the previous range is the > > + * first value of the current range. > > + */ > > +#define DRM_MODE_LUT_REUSE_LAST BIT(3) > > +/* the curve must be non-decreasing */ #define > > +DRM_MODE_LUT_NON_DECREASING BIT(4) > > +/* the curve is reflected across origin for negative inputs */ > > +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5) > > +/* the same curve (red) is used for blue and green channels as well > > +*/ #define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6) > > + > > +struct drm_color_lut_range { > > + /* DRM_MODE_LUT_* */ > > + __u32 flags; > > + /* number of points on the curve */ > > + __u16 count; > > + /* input/output bits per component */ > > + __u8 input_bpc, output_bpc; > > + /* input start/end values */ > > + __s32 start, end; > > + /* output min/max values */ > > + __s32 min, max; > > +}; > > + > > +enum lut_type { > > + LUT_TYPE_DEGAMMA = 0, > > + LUT_TYPE_GAMMA = 1, > > +}; > > + > > #define DRM_MODE_PAGE_FLIP_EVENT 0x01
Re: [PATCH 1/9] drm: Add gamma mode property
On Tue, 1 Jun 2021 16:11:27 +0530 Uma Shankar wrote: > Add a gamma mode property to enable various kind of > gamma modes supported by platforms like: Interpolated, Split, > Multi Segmented, Logarithmic etc. Userspace can get this property > and should be able to get the platform capabilities wrt various > gamma modes possible and the possible ranges. > > It can select one of the modes exposed as blob_id as an > enum and set the respective mode. > > It can then create the LUT and send it to driver using > already available GAMMA_LUT property as blob. > > Note: This is based on design by Ville and is being carried forward > based on his original idea. > > Signed-off-by: Uma Shankar > --- > drivers/gpu/drm/drm_atomic_uapi.c | 5 +++ > drivers/gpu/drm/drm_color_mgmt.c | 75 +++ > include/drm/drm_color_mgmt.h | 8 > include/drm/drm_crtc.h| 14 ++ > include/uapi/drm/drm_mode.h | 43 ++ > 5 files changed, 145 insertions(+) > ... > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 13eeba2a750a..b1eead03ebe8 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -262,6 +262,13 @@ struct drm_crtc_state { >*/ > struct drm_property_blob *mode_blob; > > + /** > + * @gamma_mode: This is a blob_id and exposes the platform capabilities > + * wrt to various gamma modes and the respective lut ranges. This also > + * helps user select a gamma mode amongst the supported ones. > + */ > + u32 gamma_mode; > + > /** >* @degamma_lut: >* > @@ -1096,6 +1103,13 @@ struct drm_crtc { >*/ > struct drm_property *scaling_filter_property; > > + /** > + * @gamma_mode_property: Optional CRTC property to enumerate and > + * select the mode of the crtc gamma/degmama LUTs. This also exposes > + * the lut ranges of the various supported gamma modes to userspace. > + */ > + struct drm_property *gamma_mode_property; > + > /** >* @state: >* > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 9b6722d45f36..d7758d351936 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -819,6 +819,49 @@ struct hdr_output_metadata { > }; > }; > > +/* > + * DRM_MODE_LUT_GAMMA|DRM_MODE_LUT_DEGAMMA is legal and means the LUT > + * can be used for either purpose, but not simultaneously. To expose > + * modes that support gamma and degamma simultaneously the gamma mode > + * must declare distinct DRM_MODE_LUT_GAMMA and DRM_MODE_LUT_DEGAMMA > + * ranges. > + */ > +/* LUT is for gamma (after CTM) */ > +#define DRM_MODE_LUT_GAMMA BIT(0) > +/* LUT is for degamma (before CTM) */ > +#define DRM_MODE_LUT_DEGAMMA BIT(1) > +/* linearly interpolate between the points */ > +#define DRM_MODE_LUT_INTERPOLATE BIT(2) > +/* > + * the last value of the previous range is the > + * first value of the current range. > + */ > +#define DRM_MODE_LUT_REUSE_LAST BIT(3) > +/* the curve must be non-decreasing */ > +#define DRM_MODE_LUT_NON_DECREASING BIT(4) > +/* the curve is reflected across origin for negative inputs */ > +#define DRM_MODE_LUT_REFLECT_NEGATIVE BIT(5) > +/* the same curve (red) is used for blue and green channels as well */ > +#define DRM_MODE_LUT_SINGLE_CHANNEL BIT(6) > + > +struct drm_color_lut_range { > + /* DRM_MODE_LUT_* */ > + __u32 flags; > + /* number of points on the curve */ > + __u16 count; > + /* input/output bits per component */ > + __u8 input_bpc, output_bpc; > + /* input start/end values */ > + __s32 start, end; > + /* output min/max values */ > + __s32 min, max; > +}; > + > +enum lut_type { > + LUT_TYPE_DEGAMMA = 0, > + LUT_TYPE_GAMMA = 1, > +}; > + > #define DRM_MODE_PAGE_FLIP_EVENT 0x01 > #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 > #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4 Hi, where is the UAPI documentation for this new GAMMA_MODE? As a userspace dev, I have no idea what to do with the above based on what's written here. Also, reading the description of DRM_CLIENT_CAP_ADVANCE_GAMMA_MODES in patch 5/9, what difference does it make whether userspace sets or does not set that cap? I don't understand the implications from the description. Thanks, pq pgpz0DTu2iUKf.pgp Description: OpenPGP digital signature