[Intel-gfx] [v11 00/12] Add HDR Metadata Parsing and handling in DRM layer

Shankar, Uma uma.shankar at intel.com
Wed May 29 17:39:31 UTC 2019 


>> >-----Original Message-----
>> >From: Daniel Vetter [mailto:daniel at ffwll.ch]
>> >Sent: Wednesday, May 29, 2019 8:33 PM
>> >To: Shankar, Uma <uma.shankar at intel.com>
>> >Cc: intel-gfx <intel-gfx at lists.freedesktop.org>; dri-devel <dri-
>> >devel at lists.freedesktop.org>; Daniele Castagna
>> ><dcastagna at chromium.org>; jonas at kwiboo.se; Sean Paul
>> ><seanpaul at chromium.org>; Sharma, Shashank
>> ><shashank.sharma at intel.com>; Syrjala, Ville
>> ><ville.syrjala at 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 at intel.com>
>wrote:
>> >>
>> >>
>> >>
>> >> >-----Original Message-----
>> >> >From: Daniel Vetter [mailto:daniel at ffwll.ch]
>> >> >Sent: Wednesday, May 29, 2019 3:13 PM
>> >> >To: Shankar, Uma <uma.shankar at intel.com>
>> >> >Cc: intel-gfx <intel-gfx at lists.freedesktop.org>; dri-devel <dri-
>> >> >devel at lists.freedesktop.org>; Daniele Castagna
>> >> ><dcastagna at chromium.org>; jonas at kwiboo.se; Sean Paul
>> >> ><seanpaul at chromium.org>; Sharma, Shashank
>> >> ><shashank.sharma at intel.com>; Syrjala, Ville
>> >> ><ville.syrjala at 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 at 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.

Sure Daniel, I will create a patch to remove those and then drop the documentation
from the static functions from this file.

>I'm also working on some patches to document drm subsystem documentation rules
>better going forward.

Yeah, this will help have some standardization. Thanks.

Regards,
Uma Shankar

>-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 at 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:
>> >> >> 1. 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.
>> >> >> 2. 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-Decemb
>> >> >> er/
>> >> >> 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 at 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 at 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

More information about the Intel-gfx mailing list