On Wed, May 29, 2019 at 7:29 PM Shankar, Uma uma.shankar@intel.com wrote:
-----Original Message----- From: Daniel Vetter [mailto:daniel@ffwll.ch] Sent: Wednesday, May 29, 2019 8:33 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx intel-gfx@lists.freedesktop.org; dri-devel <dri- devel@lists.freedesktop.org>; Daniele Castagna dcastagna@chromium.org; jonas@kwiboo.se; Sean Paul seanpaul@chromium.org; Sharma, Shashank shashank.sharma@intel.com; Syrjala, Ville ville.syrjala@linux.intel.com Subject: Re: [Intel-gfx] [v11 00/12] Add HDR Metadata Parsing and handling in DRM layer
On Wed, May 29, 2019 at 3:59 PM Shankar, Uma uma.shankar@intel.com wrote:
-----Original Message----- From: Daniel Vetter [mailto:daniel@ffwll.ch] Sent: Wednesday, May 29, 2019 3:13 PM To: Shankar, Uma uma.shankar@intel.com Cc: intel-gfx intel-gfx@lists.freedesktop.org; dri-devel <dri- devel@lists.freedesktop.org>; Daniele Castagna dcastagna@chromium.org; jonas@kwiboo.se; Sean Paul seanpaul@chromium.org; Sharma, Shashank shashank.sharma@intel.com; Syrjala, Ville ville.syrjala@linux.intel.com Subject: Re: [Intel-gfx] [v11 00/12] Add HDR Metadata Parsing and handling in DRM layer
When building the docs with make htmldocs:
./include/drm/drm_mode_config.h:841: warning: Incorrect use of kernel-doc format: * hdr_output_metadata_property: Connector property containing hdr ./include/drm/drm_mode_config.h:918: warning: Function parameter or member 'hdr_output_metadata_property' not described in 'drm_mode_config' ./include/drm/drm_connector.h:1251: warning: Function parameter or member 'hdr_output_metadata' not described in 'drm_connector' ./include/drm/drm_connector.h:1251: warning: Function parameter or member 'hdr_sink_metadata' not described in 'drm_connector'
Please fix.
Thanks Daniel, I missed to check the docs warnings. Will fix this.
In general documentation for this patch seems to be extremely lacking. No property spec, not docs for most of the new stuff added, no nothing.
Will add the property description in connector create properties as well.
Please fix asap.
Yeah, will send out the doc fix patch soon.
btw I think the hdmi infoframe helper docs also need more polish. Generally we only document the driver interface, formal kerneldoc comments for static functions is overkill. I think you added some of those.
Hi Daniel, I tried to stay consistent with how the existing stuff was handled here. So yes, it got added as part of this as well. May be I will drop it for HDR stuff without disturbing the legacy stuff. Hope this is fine ?
There's just one patch before yours that adds kerneldoc to static inline:
commit 2c676f378edb16cb68f7815581c8119fc43a4b85 Author: Martin Bugge marbugge@cisco.com Date: Fri Dec 19 09:14:21 2014 -0300
[media] hdmi: added unpack and logging functions for InfoFrames
That one didn't go through dri-devel I think. None of the other static functions are documented.
I think a patch to remove those (and maybe just have simple comments, but doesn't look like anything fancy that's not already obvious) makes sense here.
I'm also working on some patches to document drm subsystem documentation rules better going forward. -Daniel
Regards, Uma Shankar
If you feel like a comment is needed, sure do that, but just a plain comment. Always worth it to make sure that the documentation you're typing actually shows up in the output, and correctly. If it doesn't, then there's something to improve.
Can you pls also take a look at that?
Thanks, Daniel
Regards, Uma Shankar
Shashank, Ville, this is stuff reviewers must catch.
Thanks, Daniel
On Thu, May 16, 2019 at 3:43 PM Uma Shankar uma.shankar@intel.com wrote:
This patch series enables HDR support in drm. It basically defines HDR metadata structures, property to pass content (after blending) metadata from user space compositors to driver.
Dynamic Range and Mastering infoframe creation and sending.
ToDo:
- We need to get the color framework in place for all planes which support HDR content in hardware. This is already in progres and patches are out for review in mailing list.
- UserSpace/Compositors: Blending policies and metadata blob creation and passing to driver. Work is already in progress by Intel's middleware teams on wayland and the patches for the same are in review.
A POC has already been developed by Ville based on wayland. Please refer below link to see the component interactions and usage: https://lists.freedesktop.org/archives/wayland-devel/2017-December/ 036 403.html
v2: Updated Ville's POC changes to the patch series.Incorporated cleanups and fixes from Ville. Rebase on latest drm-tip.
v3: Fixed a warning causing builds to break on CI. No major change.
v4: Addressed Shashank's review comments.
v5: Rebase on top of Ville's infoframe refactoring changes. Fixed non modeset case for HDR metadata update. Dropped a redundant patch.
v6: Addressed Shashank's review comments and added RB's received.
v7: Squashed 2 patches, dropped 1 change and addressed Brian Starkey's and Shashank's review comments.
v8: Addressed Jonas Karlman review comments. Added Shashank's RB to the series, fixed a WARN_ON on BYT/CHT.
v9: Addressed Ville and Jonas Karlman's review comments. Added the infoframe state readout and metadata reference count.
v10: Addressed review comments from Jonas and Ville. Dropped one patch related to i915 fastset handling as per Ville's feedback.
v11: Addressed Ville's review comments.
Note: v9 version is already tested with Kodi and a confirmation from team kodi has been received. Branch details for the same as below: https://github.com/xbmc/xbmc/tree/feature_drmprime-vaapi
v9 of this series is: Tested-by: Jonas Karlman jonas@kwiboo.se
Jonas Karlman (1): drm: Add reference counting on HDR metadata blob
Uma Shankar (9): drm: Add HDR source metadata property drm: Parse HDR metadata info from EDID drm: Enable HDR infoframe support drm/i915: Attach HDR metadata property to connector drm/i915: Write HDR infoframe and send to panel drm/i915:Enabled Modeset when HDR Infoframe changes drm/i915: Added DRM Infoframe handling for BYT/CHT video/hdmi: Add Unpack function for DRM infoframe drm/i915: Add state readout for DRM infoframe
Ville Syrjälä (2): drm: Add HLG EOTF drm/i915: Enable infoframes on GLK+ for HDR
drivers/gpu/drm/drm_atomic_state_helper.c | 5 + drivers/gpu/drm/drm_atomic_uapi.c | 12 ++ drivers/gpu/drm/drm_connector.c | 6 + drivers/gpu/drm/drm_edid.c | 124 ++++++++++++++ drivers/gpu/drm/i915/i915_reg.h | 4 + drivers/gpu/drm/i915/intel_atomic.c | 14 +- drivers/gpu/drm/i915/intel_ddi.c | 3 + drivers/gpu/drm/i915/intel_display.c | 1 + drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_hdmi.c | 67 +++++++- drivers/video/hdmi.c | 257 ++++++++++++++++++++++++++++++ include/drm/drm_connector.h | 10 ++ include/drm/drm_edid.h | 5 + include/drm/drm_mode_config.h | 7 + include/linux/hdmi.h | 55 +++++++ include/uapi/drm/drm_mode.h | 23 +++ 16 files changed, 589 insertions(+), 5 deletions(-)
-- 1.9.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch