>> >-----Original Message----- >> >From: Daniel Vetter [mailto:daniel@xxxxxxxx] >> >Sent: Wednesday, May 29, 2019 8:33 PM >> >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >> >Cc: intel-gfx <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; dri-devel <dri- >> >devel@xxxxxxxxxxxxxxxxxxxxx>; Daniele Castagna >> ><dcastagna@xxxxxxxxxxxx>; jonas@xxxxxxxxx; Sean Paul >> ><seanpaul@xxxxxxxxxxxx>; Sharma, Shashank >> ><shashank.sharma@xxxxxxxxx>; Syrjala, Ville >> ><ville.syrjala@xxxxxxxxxxxxxxx> >> >Subject: Re: [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@xxxxxxxxx> >wrote: >> >> >> >> >> >> >> >> >-----Original Message----- >> >> >From: Daniel Vetter [mailto:daniel@xxxxxxxx] >> >> >Sent: Wednesday, May 29, 2019 3:13 PM >> >> >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >> >> >Cc: intel-gfx <intel-gfx@xxxxxxxxxxxxxxxxxxxxx>; dri-devel <dri- >> >> >devel@xxxxxxxxxxxxxxxxxxxxx>; Daniele Castagna >> >> ><dcastagna@xxxxxxxxxxxx>; jonas@xxxxxxxxx; Sean Paul >> >> ><seanpaul@xxxxxxxxxxxx>; Sharma, Shashank >> >> ><shashank.sharma@xxxxxxxxx>; Syrjala, Ville >> >> ><ville.syrjala@xxxxxxxxxxxxxxx> >> >> >Subject: Re: [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@xxxxxxxxx> >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@xxxxxxxxx> >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@xxxxxxxxx> >> >> >> >> >> >> 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@xxxxxxxxxxxxxxxxxxxxx >> >> >> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
