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 at lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
