| Message ID | fa45df6ae2d7dbf546b56418512e399226c753fa.1464207811.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25 May 2016 at 21:43, Jyri Sarha <jsarha@ti.com> wrote: > Add drm_crtc_enable_color_mgmt() to. The new function makes the old > drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more > flexible. It allows driver to enable only the feature it has without > forcing to enable all three color management features: gegamma lut, csc > matrix (ctm), and gamma lut. > Why don't we just update the existing one ? In one step (patch) or two - a) don't register property if respective _lut_size is zero b) bring in has_ctm. Regards, Emil
On 25 May 2016 at 22:05, Emil Velikov <emil.l.velikov@gmail.com> wrote: > On 25 May 2016 at 21:43, Jyri Sarha <jsarha@ti.com> wrote: >> Add drm_crtc_enable_color_mgmt() to. The new function makes the old >> drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more >> flexible. It allows driver to enable only the feature it has without >> forcing to enable all three color management features: gegamma lut, csc >> matrix (ctm), and gamma lut. >> > Why don't we just update the existing one ? In one step (patch) or two > - a) don't register property if respective _lut_size is zero b) bring > in has_ctm. > Ouch I missed that the goal here is to move the function out of drm_crtc_helper.c. Sorry about that. Perhaps the commit message can be reworded a bit - something like the following comes to mind. "Introduce drm_crtc_enable_color_mgmt() function which supersedes the crtc_helper one (xxx: add name). The latter always creates all properties (xxx: list props) and is not really a helper. The new version allows explicit control of the properties created as required by some hardware/drivers (xxx: name driver)." Feel free to reuse and/tweak. Thanks Emil
On Wed, May 25, 2016 at 10:19:50PM +0100, Emil Velikov wrote: > On 25 May 2016 at 22:05, Emil Velikov <emil.l.velikov@gmail.com> wrote: > > On 25 May 2016 at 21:43, Jyri Sarha <jsarha@ti.com> wrote: > >> Add drm_crtc_enable_color_mgmt() to. The new function makes the old > >> drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more > >> flexible. It allows driver to enable only the feature it has without > >> forcing to enable all three color management features: gegamma lut, csc > >> matrix (ctm), and gamma lut. > >> > > Why don't we just update the existing one ? In one step (patch) or two > > - a) don't register property if respective _lut_size is zero b) bring > > in has_ctm. > > > Ouch I missed that the goal here is to move the function out of > drm_crtc_helper.c. Sorry about that. The goal still was to move it, and update the existing callers. Adding a new without removing the old one is not what I suggested really ;-) -Daniel > > Perhaps the commit message can be reworded a bit - something like the > following comes to mind. > > "Introduce drm_crtc_enable_color_mgmt() function which supersedes the > crtc_helper one (xxx: add name). The latter always creates all > properties (xxx: list props) and is not really a helper. > The new version allows explicit control of the properties created as > required by some hardware/drivers (xxx: name driver)." > > Feel free to reuse and/tweak. > > Thanks > Emil > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d2a6d95..b097226 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -6065,3 +6065,48 @@ struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev, return tg; } EXPORT_SYMBOL(drm_mode_create_tile_group); + +/** + * drm_crtc_enable_color_mgmt - enable color management properties + * @crtc: DRM CRTC + * @degamma_lut_size: the size of the degamma lut (before CSC) + * @has_ctm: whether to attach ctm_property for CSC matrix + * @gamma_lut_size: the size of the gamma lut (after CSC) + * + * This function lets the driver enable the color correction + * properties on a CRTC. This includes 3 degamma, csc and gamma + * properties that userspace can set and 2 size properties to inform + * the userspace of the lut sizes. Each of the properties are + * optional. The gamma and degamma properties are only attached if + * their size is not 0 and ctm_property is only attached if has_ctm is + * true. + */ +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 = crtc->dev; + struct drm_mode_config *config = &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); + } +} +EXPORT_SYMBOL(drm_crtc_enable_color_mgmt); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index d1559cd..36d3bbf 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2553,7 +2553,10 @@ extern struct drm_property *drm_mode_create_rotation_property(struct drm_device unsigned int supported_rotations); extern unsigned int drm_rotation_simplify(unsigned int rotation, unsigned int supported_rotations); - +extern void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, + uint degamma_lut_size, + bool has_ctm, + uint gamma_lut_size); /* Helpers */ static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
Add drm_crtc_enable_color_mgmt() to. The new function makes the old drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more flexible. It allows driver to enable only the feature it has without forcing to enable all three color management features: gegamma lut, csc matrix (ctm), and gamma lut. Suggested-by: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Jyri Sarha <jsarha@ti.com> --- drivers/gpu/drm/drm_crtc.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 5 ++++- 2 files changed, 49 insertions(+), 1 deletion(-)