| Message ID | 1442425040-32185-10-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 16, 2015 at 11:07:06PM +0530, Shashank Sharma wrote: > DRM color manager contains these color properties: > > 1. "crtc_palette_capabilities_property": to allow a > core driver to load and showcase its color correction > capabilities to user space. > 2. "ctm": Color transformation matrix property, where a > color transformation matrix of 9 correction values gets > applied as correction. > 3. "palette_before_ctm": for corrections which get applied > beore color transformation matrix correction. > 4. "palette_after_ctm": for corrections which get applied > after color transformation matrix correction. > > Intel color manager registers: > 1. Gamma correction property as "palette_after_ctm" property > 2. Degamma correction capability as "palette_bafore_ctm" property > capability as "palette_after_ctm" DRM color property hook. > 3. CSC as "ctm" property. > > This patch does the following: > 1. Add a function which loads the platform's color correction > capabilities in the cm_crtc_palette_capabilities_property structure. > 2. Attaches the cm_crtc_palette_capabilities_property to every CRTC > getting initiaized. > 3. Adds two new parameters "num_samples_after_ctm" and > "num_samples_before_ctm" in intel_device_info as gamma and > degamma coefficients vary per platform basis. > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_color_manager.c | 45 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_color_manager.h | 2 ++ > 3 files changed, 49 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 3bf8a9b..22de2cb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -798,6 +798,8 @@ struct intel_device_info { > u8 num_sprites[I915_MAX_PIPES]; > u8 gen; > u8 ring_mask; /* Rings supported by the HW */ > + u16 num_samples_after_ctm; > + u16 num_samples_before_ctm; > DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON); > /* Register offsets for the various display pipes and transcoders */ > int pipe_offsets[I915_MAX_TRANSCODERS]; > diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c > index 7357d99..77f58f2 100644 > --- a/drivers/gpu/drm/i915/intel_color_manager.c > +++ b/drivers/gpu/drm/i915/intel_color_manager.c > @@ -27,7 +27,52 @@ > > #include "intel_color_manager.h" > > +int get_pipe_capabilities(struct drm_device *dev, > + struct drm_palette_caps *palette_caps, struct drm_crtc *crtc) > +{ > + struct drm_property_blob *blob; > + > + /* > + * This function loads best capability for gamma correction > + * For example: > + * CHV best Gamma correction (CGM unit, 10 bit) > + * has 257 entries, best degamma is 65 entries > + */ > + palette_caps->version = COLOR_STRUCT_VERSION; > + palette_caps->num_samples_after_ctm = > + INTEL_INFO(dev)->num_samples_after_ctm; > + palette_caps->num_samples_before_ctm = > + INTEL_INFO(dev)->num_samples_before_ctm; > + blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps), > + (const void *) palette_caps); > + if (IS_ERR_OR_NULL(blob)) { > + DRM_ERROR("Create blob for capabilities failed\n"); > + return PTR_ERR(blob); > + } > + > + return blob->base.id; > +} > + > void intel_attach_color_properties_to_crtc(struct drm_device *dev, > struct drm_mode_object *mode_obj) > { > + struct drm_mode_config *config = &dev->mode_config; > + struct drm_palette_caps *palette_caps; > + struct drm_crtc *crtc; > + int capabilities_blob_id; > + > + crtc = obj_to_crtc(mode_obj); > + palette_caps = kzalloc(sizeof(struct drm_palette_caps), > + GFP_KERNEL); > + capabilities_blob_id = get_pipe_capabilities(dev, > + palette_caps, crtc); > + > + if (config->cm_crtc_palette_capabilities_property) { > + drm_object_attach_property(mode_obj, > + config->cm_crtc_palette_capabilities_property, > + capabilities_blob_id); I didn't find any code to attach the before/after_ctm gamma properties and the ctm property. Also I think a small helper would be really nice here in the drm core: drm_crtc_attach_cc_properties(struct drm_crtc *crtc, int gamma_before_ctm, bool ctm, int gamm_after_ctm) And then that directly constructs the palette caps from the given values (if they're non-0) and also attaches the properties to the crtc (if they're non-0 for gamma, treu for the ctm). If that attach_property code really isn't there I wonder a bit how this all works, we /should/ be filtering out properties which aren't attached to the object. -Daniel > + DRM_DEBUG_DRIVER("Capabilities attached to CRTC\n"); > + } > + > + kfree(palette_caps); > } > diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > index 04c921d..1a42244 100644 > --- a/drivers/gpu/drm/i915/intel_color_manager.h > +++ b/drivers/gpu/drm/i915/intel_color_manager.h > @@ -27,3 +27,5 @@ > #include <drm/drmP.h> > #include <drm/drm_crtc_helper.h> > #include "i915_drv.h" > + > +#define COLOR_STRUCT_VERSION 1 > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Regards Shashank On 9/22/2015 6:54 PM, Daniel Vetter wrote: > On Wed, Sep 16, 2015 at 11:07:06PM +0530, Shashank Sharma wrote: >> DRM color manager contains these color properties: >> >> 1. "crtc_palette_capabilities_property": to allow a >> core driver to load and showcase its color correction >> capabilities to user space. >> 2. "ctm": Color transformation matrix property, where a >> color transformation matrix of 9 correction values gets >> applied as correction. >> 3. "palette_before_ctm": for corrections which get applied >> beore color transformation matrix correction. >> 4. "palette_after_ctm": for corrections which get applied >> after color transformation matrix correction. >> >> Intel color manager registers: >> 1. Gamma correction property as "palette_after_ctm" property >> 2. Degamma correction capability as "palette_bafore_ctm" property >> capability as "palette_after_ctm" DRM color property hook. >> 3. CSC as "ctm" property. >> >> This patch does the following: >> 1. Add a function which loads the platform's color correction >> capabilities in the cm_crtc_palette_capabilities_property structure. >> 2. Attaches the cm_crtc_palette_capabilities_property to every CRTC >> getting initiaized. >> 3. Adds two new parameters "num_samples_after_ctm" and >> "num_samples_before_ctm" in intel_device_info as gamma and >> degamma coefficients vary per platform basis. >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 2 ++ >> drivers/gpu/drm/i915/intel_color_manager.c | 45 ++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_color_manager.h | 2 ++ >> 3 files changed, 49 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 3bf8a9b..22de2cb 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -798,6 +798,8 @@ struct intel_device_info { >> u8 num_sprites[I915_MAX_PIPES]; >> u8 gen; >> u8 ring_mask; /* Rings supported by the HW */ >> + u16 num_samples_after_ctm; >> + u16 num_samples_before_ctm; >> DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON); >> /* Register offsets for the various display pipes and transcoders */ >> int pipe_offsets[I915_MAX_TRANSCODERS]; >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c >> index 7357d99..77f58f2 100644 >> --- a/drivers/gpu/drm/i915/intel_color_manager.c >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c >> @@ -27,7 +27,52 @@ >> >> #include "intel_color_manager.h" >> >> +int get_pipe_capabilities(struct drm_device *dev, >> + struct drm_palette_caps *palette_caps, struct drm_crtc *crtc) >> +{ >> + struct drm_property_blob *blob; >> + >> + /* >> + * This function loads best capability for gamma correction >> + * For example: >> + * CHV best Gamma correction (CGM unit, 10 bit) >> + * has 257 entries, best degamma is 65 entries >> + */ >> + palette_caps->version = COLOR_STRUCT_VERSION; >> + palette_caps->num_samples_after_ctm = >> + INTEL_INFO(dev)->num_samples_after_ctm; >> + palette_caps->num_samples_before_ctm = >> + INTEL_INFO(dev)->num_samples_before_ctm; >> + blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps), >> + (const void *) palette_caps); >> + if (IS_ERR_OR_NULL(blob)) { >> + DRM_ERROR("Create blob for capabilities failed\n"); >> + return PTR_ERR(blob); >> + } >> + >> + return blob->base.id; >> +} >> + >> void intel_attach_color_properties_to_crtc(struct drm_device *dev, >> struct drm_mode_object *mode_obj) >> { >> + struct drm_mode_config *config = &dev->mode_config; >> + struct drm_palette_caps *palette_caps; >> + struct drm_crtc *crtc; >> + int capabilities_blob_id; >> + >> + crtc = obj_to_crtc(mode_obj); >> + palette_caps = kzalloc(sizeof(struct drm_palette_caps), >> + GFP_KERNEL); >> + capabilities_blob_id = get_pipe_capabilities(dev, >> + palette_caps, crtc); >> + >> + if (config->cm_crtc_palette_capabilities_property) { >> + drm_object_attach_property(mode_obj, >> + config->cm_crtc_palette_capabilities_property, >> + capabilities_blob_id); > > I didn't find any code to attach the before/after_ctm gamma properties and > the ctm property. Also I think a small helper would be really nice here in > the drm core: > Patch 15,16,17 adds those for gamma, degamma and CSC. This is just for the capabilities property. > drm_crtc_attach_cc_properties(struct drm_crtc *crtc, > int gamma_before_ctm, > bool ctm, > int gamm_after_ctm) > > And then that directly constructs the palette caps from the given values > (if they're non-0) and also attaches the properties to the crtc (if > they're non-0 for gamma, treu for the ctm). > > If that attach_property code really isn't there I wonder a bit how this > all works, we /should/ be filtering out properties which aren't attached > to the object. As I mentioned in the cover-letter, here is the sequence: First 5 patches add color management support in DRM layer Next 7 patches add Intel color management framework in I915 driver (This is still the framework part) Next 5 patches add color correction for CHV (gamma, degamma and CSC) (Here comes the core property and enabling part) Next 2 patches enable color management, by attaching the properties to CRTC(Matt's comment on enable property code only when the job is done) Next 4 patches add color correction for BDW (gamma, degamma) > -Daniel > >> + DRM_DEBUG_DRIVER("Capabilities attached to CRTC\n"); >> + } >> + >> + kfree(palette_caps); >> } >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h >> index 04c921d..1a42244 100644 >> --- a/drivers/gpu/drm/i915/intel_color_manager.h >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h >> @@ -27,3 +27,5 @@ >> #include <drm/drmP.h> >> #include <drm/drm_crtc_helper.h> >> #include "i915_drv.h" >> + >> +#define COLOR_STRUCT_VERSION 1 >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Wed, Sep 23, 2015 at 02:05:25PM +0530, Sharma, Shashank wrote: > Regards > Shashank > > On 9/22/2015 6:54 PM, Daniel Vetter wrote: > >On Wed, Sep 16, 2015 at 11:07:06PM +0530, Shashank Sharma wrote: > >>DRM color manager contains these color properties: > >> > >>1. "crtc_palette_capabilities_property": to allow a > >>core driver to load and showcase its color correction > >>capabilities to user space. > >>2. "ctm": Color transformation matrix property, where a > >>color transformation matrix of 9 correction values gets > >>applied as correction. > >>3. "palette_before_ctm": for corrections which get applied > >>beore color transformation matrix correction. > >>4. "palette_after_ctm": for corrections which get applied > >>after color transformation matrix correction. > >> > >>Intel color manager registers: > >>1. Gamma correction property as "palette_after_ctm" property > >>2. Degamma correction capability as "palette_bafore_ctm" property > >>capability as "palette_after_ctm" DRM color property hook. > >>3. CSC as "ctm" property. > >> > >>This patch does the following: > >>1. Add a function which loads the platform's color correction > >>capabilities in the cm_crtc_palette_capabilities_property structure. > >>2. Attaches the cm_crtc_palette_capabilities_property to every CRTC > >>getting initiaized. > >>3. Adds two new parameters "num_samples_after_ctm" and > >>"num_samples_before_ctm" in intel_device_info as gamma and > >>degamma coefficients vary per platform basis. > >> > >>Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > >>--- > >> drivers/gpu/drm/i915/i915_drv.h | 2 ++ > >> drivers/gpu/drm/i915/intel_color_manager.c | 45 ++++++++++++++++++++++++++++++ > >> drivers/gpu/drm/i915/intel_color_manager.h | 2 ++ > >> 3 files changed, 49 insertions(+) > >> > >>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >>index 3bf8a9b..22de2cb 100644 > >>--- a/drivers/gpu/drm/i915/i915_drv.h > >>+++ b/drivers/gpu/drm/i915/i915_drv.h > >>@@ -798,6 +798,8 @@ struct intel_device_info { > >> u8 num_sprites[I915_MAX_PIPES]; > >> u8 gen; > >> u8 ring_mask; /* Rings supported by the HW */ > >>+ u16 num_samples_after_ctm; > >>+ u16 num_samples_before_ctm; > >> DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON); > >> /* Register offsets for the various display pipes and transcoders */ > >> int pipe_offsets[I915_MAX_TRANSCODERS]; > >>diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c > >>index 7357d99..77f58f2 100644 > >>--- a/drivers/gpu/drm/i915/intel_color_manager.c > >>+++ b/drivers/gpu/drm/i915/intel_color_manager.c > >>@@ -27,7 +27,52 @@ > >> > >> #include "intel_color_manager.h" > >> > >>+int get_pipe_capabilities(struct drm_device *dev, > >>+ struct drm_palette_caps *palette_caps, struct drm_crtc *crtc) > >>+{ > >>+ struct drm_property_blob *blob; > >>+ > >>+ /* > >>+ * This function loads best capability for gamma correction > >>+ * For example: > >>+ * CHV best Gamma correction (CGM unit, 10 bit) > >>+ * has 257 entries, best degamma is 65 entries > >>+ */ > >>+ palette_caps->version = COLOR_STRUCT_VERSION; > >>+ palette_caps->num_samples_after_ctm = > >>+ INTEL_INFO(dev)->num_samples_after_ctm; > >>+ palette_caps->num_samples_before_ctm = > >>+ INTEL_INFO(dev)->num_samples_before_ctm; > >>+ blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps), > >>+ (const void *) palette_caps); > >>+ if (IS_ERR_OR_NULL(blob)) { > >>+ DRM_ERROR("Create blob for capabilities failed\n"); > >>+ return PTR_ERR(blob); > >>+ } > >>+ > >>+ return blob->base.id; > >>+} > >>+ > >> void intel_attach_color_properties_to_crtc(struct drm_device *dev, > >> struct drm_mode_object *mode_obj) > >> { > >>+ struct drm_mode_config *config = &dev->mode_config; > >>+ struct drm_palette_caps *palette_caps; > >>+ struct drm_crtc *crtc; > >>+ int capabilities_blob_id; > >>+ > >>+ crtc = obj_to_crtc(mode_obj); > >>+ palette_caps = kzalloc(sizeof(struct drm_palette_caps), > >>+ GFP_KERNEL); > >>+ capabilities_blob_id = get_pipe_capabilities(dev, > >>+ palette_caps, crtc); > >>+ > >>+ if (config->cm_crtc_palette_capabilities_property) { > >>+ drm_object_attach_property(mode_obj, > >>+ config->cm_crtc_palette_capabilities_property, > >>+ capabilities_blob_id); > > > >I didn't find any code to attach the before/after_ctm gamma properties and > >the ctm property. Also I think a small helper would be really nice here in > >the drm core: > > > Patch 15,16,17 adds those for gamma, degamma and CSC. This is just for the > capabilities property. Indeed, my mail search failed me. > >drm_crtc_attach_cc_properties(struct drm_crtc *crtc, > > int gamma_before_ctm, > > bool ctm, > > int gamm_after_ctm) > > > >And then that directly constructs the palette caps from the given values > >(if they're non-0) and also attaches the properties to the crtc (if > >they're non-0 for gamma, treu for the ctm). > > > >If that attach_property code really isn't there I wonder a bit how this > >all works, we /should/ be filtering out properties which aren't attached > >to the object. > As I mentioned in the cover-letter, here is the sequence: > First 5 patches add color management support in DRM layer > Next 7 patches add Intel color management framework in I915 driver (This is > still the framework part) > Next 5 patches add color correction for CHV (gamma, degamma and CSC) > (Here comes the core property and enabling part) > Next 2 patches enable color management, by attaching the properties to > CRTC(Matt's comment on enable property code only when the job is done) > Next 4 patches add color correction for BDW (gamma, degamma) Yeah I just missed this, no idea why the search in my mail client didn't turn up patches 15-17. I still think drm_crtc_attach_cc_properties in the drm core would be good, instead of splitting part of the color manager core support around the properties between the driver and core like you do here. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3bf8a9b..22de2cb 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -798,6 +798,8 @@ struct intel_device_info { u8 num_sprites[I915_MAX_PIPES]; u8 gen; u8 ring_mask; /* Rings supported by the HW */ + u16 num_samples_after_ctm; + u16 num_samples_before_ctm; DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON); /* Register offsets for the various display pipes and transcoders */ int pipe_offsets[I915_MAX_TRANSCODERS]; diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c index 7357d99..77f58f2 100644 --- a/drivers/gpu/drm/i915/intel_color_manager.c +++ b/drivers/gpu/drm/i915/intel_color_manager.c @@ -27,7 +27,52 @@ #include "intel_color_manager.h" +int get_pipe_capabilities(struct drm_device *dev, + struct drm_palette_caps *palette_caps, struct drm_crtc *crtc) +{ + struct drm_property_blob *blob; + + /* + * This function loads best capability for gamma correction + * For example: + * CHV best Gamma correction (CGM unit, 10 bit) + * has 257 entries, best degamma is 65 entries + */ + palette_caps->version = COLOR_STRUCT_VERSION; + palette_caps->num_samples_after_ctm = + INTEL_INFO(dev)->num_samples_after_ctm; + palette_caps->num_samples_before_ctm = + INTEL_INFO(dev)->num_samples_before_ctm; + blob = drm_property_create_blob(dev, sizeof(struct drm_palette_caps), + (const void *) palette_caps); + if (IS_ERR_OR_NULL(blob)) { + DRM_ERROR("Create blob for capabilities failed\n"); + return PTR_ERR(blob); + } + + return blob->base.id; +} + void intel_attach_color_properties_to_crtc(struct drm_device *dev, struct drm_mode_object *mode_obj) { + struct drm_mode_config *config = &dev->mode_config; + struct drm_palette_caps *palette_caps; + struct drm_crtc *crtc; + int capabilities_blob_id; + + crtc = obj_to_crtc(mode_obj); + palette_caps = kzalloc(sizeof(struct drm_palette_caps), + GFP_KERNEL); + capabilities_blob_id = get_pipe_capabilities(dev, + palette_caps, crtc); + + if (config->cm_crtc_palette_capabilities_property) { + drm_object_attach_property(mode_obj, + config->cm_crtc_palette_capabilities_property, + capabilities_blob_id); + DRM_DEBUG_DRIVER("Capabilities attached to CRTC\n"); + } + + kfree(palette_caps); } diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h index 04c921d..1a42244 100644 --- a/drivers/gpu/drm/i915/intel_color_manager.h +++ b/drivers/gpu/drm/i915/intel_color_manager.h @@ -27,3 +27,5 @@ #include <drm/drmP.h> #include <drm/drm_crtc_helper.h> #include "i915_drv.h" + +#define COLOR_STRUCT_VERSION 1
DRM color manager contains these color properties: 1. "crtc_palette_capabilities_property": to allow a core driver to load and showcase its color correction capabilities to user space. 2. "ctm": Color transformation matrix property, where a color transformation matrix of 9 correction values gets applied as correction. 3. "palette_before_ctm": for corrections which get applied beore color transformation matrix correction. 4. "palette_after_ctm": for corrections which get applied after color transformation matrix correction. Intel color manager registers: 1. Gamma correction property as "palette_after_ctm" property 2. Degamma correction capability as "palette_bafore_ctm" property capability as "palette_after_ctm" DRM color property hook. 3. CSC as "ctm" property. This patch does the following: 1. Add a function which loads the platform's color correction capabilities in the cm_crtc_palette_capabilities_property structure. 2. Attaches the cm_crtc_palette_capabilities_property to every CRTC getting initiaized. 3. Adds two new parameters "num_samples_after_ctm" and "num_samples_before_ctm" in intel_device_info as gamma and degamma coefficients vary per platform basis. Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_color_manager.c | 45 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 2 ++ 3 files changed, 49 insertions(+)