Re: [Intel-gfx] [PATCH 00/23] Color Management for DRM

["Sharma, Shashank" <shashank.sharma@intel.com>]

Regards
Shashank

On 9/22/2015 6:57 PM, Daniel Vetter wrote:
> On Wed, Sep 16, 2015 at 11:06:57PM +0530, Shashank Sharma wrote:
>> This patch set adds Color Manager implementation in DRM layer. Color Manager
>> is an extension in DRM framework to support color correction/enhancement.
>>
>> Various Hardware platforms can support several color correction capabilities.
>> Color Manager provides abstraction of these capabilities and allows a user
>> space UI agent to correct/enhance the display using the DRM property interface.
>>
>> How is this going to work?
>> ==========================
>> 1. This patch series adds a few new properties in DRM framework. These properties are:
>>          a. color_capabilities property (type blob)
>>          b. Color Transformation Matrix property for corrections like CSC (called CTM, type blob)
>>          c. Palette correction properties for corrections like gamma fixup (called palette_correction, type blob)
>> 2. Also, this patch series adds few structures to indicate specifications of a property like size, no_of_samples for correction etc.
>> 3. These properties are present in mode_config.
>> 4. When the platform's display driver loads, it fills up the values of color_capabilities property using the standard structures (added in step 2).
>>     For example, Intel's I915 driver adds following color correction capabilities:
>>          a. gamma correction capability as palette correction property, with 257 correction coefficients and a max/min value
>>          b. csc correction capability as CTM correction property, with 3x3 transformation matrix values and max/min values
>> 5. Now when userspace comes up, it queries the platform's color capabilities by doing a get_property() on color_capabilities DRM property
>> 6. Reading the blob, the userspace understands the color capabilities of the platform.
>>     For example, userspace will understand it can support:
>>          a. palette_correction with 257 coefficients
>>          b. CSC correction with 3x3 = 9 values
>> 7. To set color correction values, userspace:
>>          a. creates a blob using the create_blob_ioctl in standard palette_correction structure format, with the correction values
>>          b. calls the set_property_ioctl with the blob_id as value for the property
>> 8. Driver refers to the blob, gets the correction values and applies the correction in HW.
>> 9. To get currently applied color correction values, userspace:
>>          a. calls a get_property_ioctl on that color property
>>          b. gets the blob_id for the currently applied correction from DRM infrastructure
>>          c. gets the blob using get_blob_ioctl and hence the currently applied values
>>
>> That's all! :)
>>
>> About the patch series:
>> =======================
>> The patch series first adds the color management support in DRM layer.
>> Then it adds the color management framework in I915 layer.
>> After that, it implements platform specific core color correction functios.
>>
>> Intel color manager registers color correction with DRM color manager in this way:
>>   - CSC transformation is registered as CTM DRM property
>>   - Gamma correction is registered as palette_after_ctm DRM property
>>   - Degamma correction is registered as palette_before_ctm DRM property
>>
>> Our thanks to all the reviewers who have given valuable comments in terms
>> of design and implementation to our previous sets of patches. Special mention
>> of thanks should go to Matt Roper for all his inputs/suggestions in implementation
>> of this module, using DRM atomic CRTC commit path.
>>
>> V2: Worked on review comments from Matt, Jim, Thierry, Rob.
>> V3: Worked on review comments from Matt, Jim, Rob:
>>   - Jim, Rob:
>>     ========
>>     Re-arranged the whole patch series in the sequence of features, currently:
>>     First 5 patches add color management support in DRM layer
>>     Next 7 patches add Intel color management framework in I915 driver
>>     Next 5 patches add color correction for CHV (gamma, degamma and CSC)
>>     Next 2 patches enable color management, by attaching the properties to CRTC(Matt)
>>     Next 4 patches add color correction for BDW (gamma, degamma)
>>   - Matt:
>>     =====
>>     Patch 3: Added refernce/unreference for blob
>>     Patch 7: return -EINVAL and added debug message
>>     Patch 8: check for valid blob, from create blob
>>              moved call to intel_crtc_attach_color_prop in the end of full implementation (CHV)
>>     Patch 9: DRM_ERROR->DRM_DEBUG for NULL blob case
>>     Patch 13: Added static for internal functions
>>     Patch 20-24: renamed gen9_* functions to bdw_*
>>     Added new variables in device_info structure num_samples_after_ctm and num_samples_before_ctm
>>     Added new function in patch 8 to load capabilities based on device_info across all platforms
>
> Ok I finally got around to look at the kernel/userspace and drm core parts
> in detail. There's a few minor bits to polish but nothing serious I think,
> looks good overall (but I didn't really look at the i915 side).
>
> One thing though is the kerneldoc part. You have some great detailed
> comments in the structs about how this is all supposed to be used, but
> afaict they're not pulled into the kerneldoc anywhere. Also we don't have
> an overview section, but if you add the drm_crtc_attach_cc_properties
> function like I suggested we could add that there (with links to all the
> various structures).
>
Now, after the explanation in the corresponding patch, do you still find 
a need for the function ?
> There's also the giant drm props table but that one is a mess. My
> suggestion would be to start a new DOC: section just for color correction,
> with an integrated markdown table. Then add that as a subsection to KMS
> property documentation. That way we'll have solid docs for this KMS
> extension (something we should have for all the other stuff like rotation
> or zpos too).
>
Yes, sure, I will do this.
> Thanks, Daniel
>
>>
>> Shashank Sharma (23):
>>    drm: Create Color Management DRM properties
>>    drm: Add structure for querying palette color capabilities
>>    drm: Add color correction blobs in CRTC state
>>    drm: Add drm structures for palette color property
>>    drm: Add structure to set/get a CTM color property
>>    drm/i915: Add atomic set property interface for CRTC
>>    drm/i915: Add atomic get property interface for CRTC
>>    drm/i915: Create color management files
>>    drm/i915: Register pipe color capabilities
>>    drm/i915: Add gamma correction handlers
>>    drm/i915: Add pipe deGamma correction handlers
>>    drm/i915: Add pipe CSC correction handlers
>>    drm/i915: CHV: Load gamma color correction values
>>    drm/i915: CHV: Pipe level Gamma correction
>>    drm/i915: CHV: Load degamma color correction values
>>    drm/i915: CHV: Pipe level degamma correction
>>    drm/i915: CHV: Pipe level CSC correction
>>    drm/i915: Commit color changes to CRTC
>>    drm/i915: Attach color properties to CRTC
>>    drm/i915: BDW: Load gamma correction values
>>    drm/i915: BDW: Pipe level Gamma correction
>>    drm/i915: BDW: Load degamma correction values
>>    drm/i915: BDW: Pipe level degamma correction
>>
>>   drivers/gpu/drm/drm_atomic_helper.c        |  12 +
>>   drivers/gpu/drm/drm_crtc.c                 |  26 +
>>   drivers/gpu/drm/i915/Makefile              |   3 +-
>>   drivers/gpu/drm/i915/i915_drv.c            |  17 +
>>   drivers/gpu/drm/i915/i915_drv.h            |   2 +
>>   drivers/gpu/drm/i915/i915_reg.h            |  41 +-
>>   drivers/gpu/drm/i915/intel_atomic.c        |  56 ++
>>   drivers/gpu/drm/i915/intel_color_manager.c | 852 +++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_color_manager.h |  99 ++++
>>   drivers/gpu/drm/i915/intel_display.c       |   6 +
>>   drivers/gpu/drm/i915/intel_drv.h           |  22 +
>>   include/drm/drm_crtc.h                     |  11 +
>>   include/uapi/drm/drm.h                     |  50 ++
>>   13 files changed, 1195 insertions(+), 2 deletions(-)
>>   create mode 100644 drivers/gpu/drm/i915/intel_color_manager.c
>>   create mode 100644 drivers/gpu/drm/i915/intel_color_manager.h
>>
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel