Re: [PATCH] drm: Fix docbook warnings in hdr metadata helper structures
From: Daniel Vetter <hidden>
Date: 2019-05-29 15:01:07
Also in:
dri-devel, intel-gfx
On Wed, May 29, 2019 at 4:16 PM Uma Shankar [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Fixes the following warnings: ./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' Also adds some property documentation for HDR Metadata Connector Property in connector property create function. Cc: Shashank Sharma <redacted> Cc: Ville Syrj채l채 <redacted> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <redacted> Cc: Sean Paul <sean@poorly.run> Cc: David Airlie <redacted> Cc: Daniel Vetter <redacted> Cc: Bartlomiej Zolnierkiewicz <redacted> Cc: "Ville Syrj채l채" <redacted> Cc: Hans Verkuil <redacted> Cc: dri-devel@lists.freedesktop.org Cc: linux-fbdev@vger.kernel.org Signed-off-by: Uma Shankar <redacted> --- drivers/gpu/drm/drm_connector.c | 8 ++++++++ include/drm/drm_connector.h | 3 ++- include/drm/drm_mode_config.h | 2 +- include/linux/hdmi.h | 1 + 4 files changed, 12 insertions(+), 2 deletions(-)diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index c9ac8b9..702307c 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c@@ -1003,6 +1003,14 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info, * can also expose this property to external outputs, in which case they * must support "None", which should be the default (since external screens * have a built-in scaler). + * + * HDR_OUTPUT_METADATA: + * Connector property to enable userspace to send HDR Metadata to driver. + * This metadata is based on the composition and blending policies decided + * by user, taking into account the hardware and sink capabilties. + * The driver gets this metadata and creates a Dynamic Range and Mastering + * Infoframe (DRM) which is then sent to sink. This notifies the sink of + * the upcoming frame's Color Encoding and Luminance parameters. */
Assuming I'm applying this correctly your adding this to the "lcd panel properties" section. That doesn't make sense to me. I think we already have a section for hdmi properties somewhere, would fit better there. This should also contain a bit more about how this is supposed to work, how it's set up from a driver pov (sprinkle links all over it) and how userspace it supposed to use it. I think since this is a using a rather complicated struct I think we need to fully document that structure too. Atm uapi/drm_mode.h isn't pulled into anywhere, so we need to fix that (a new chapter titled "Userspace API Structures" in drm-uapi.rst would be good, cross-links will work).
quoted hunk ↗ jump to hunk
int drm_connector_create_standard_properties(struct drm_device *dev)diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index f8f4003..f226ef0 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h@@ -1244,8 +1244,9 @@ struct drm_connector { */ struct llist_node free_node; - /* HDR metdata */ + /** @hdr_output_metadata: HDR Metadata to be sent to sink */ struct hdr_output_metadata hdr_output_metadata;
Uh, is this even used? It would be a bug if so, since the state userspace can set must be stored in drm_connector_state, not in drm_connector. Only read-only stuff can be in there. Please don't just blindly type docs, try to make sure that what you're documenting actually makes sense. Also, should have been a clear sign that you've forgotten to document one of the properties in the enumeration above. -Daniel
quoted hunk ↗ jump to hunk
+ /** @hdr_sink_metadata: HDR Metadata Information read from sink */ struct hdr_sink_metadata hdr_sink_metadata; };diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 4f88cc9..0b180e0 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h@@ -837,7 +837,7 @@ struct drm_mode_config { struct drm_property *writeback_out_fence_ptr_property; /** - * hdr_output_metadata_property: Connector property containing hdr + * @hdr_output_metadata_property: Connector property containing hdr * metatda. This will be provided by userspace compositors based * on HDR content */diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index ee55ba5..ea5858e 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h@@ -398,6 +398,7 @@ ssize_t hdmi_vendor_infoframe_pack_only(const struct hdmi_vendor_infoframe *fram * @spd: spd infoframe * @vendor: union of all vendor infoframes * @audio: audio infoframe + * @drm: DRM infoframe * * This is used by the generic pack function. This works since all infoframes * have the same header which also indicates which type of infoframe should be --1.9.1
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch