On Wed, May 29, 2019 at 4:16 PM Uma Shankar <uma.shankar@xxxxxxxxx> wrote:
>
> 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 <shashank.sharma@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> Cc: Sean Paul <sean@xxxxxxxxxx>
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx>
> Cc: "Ville Syrjälä" <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Hans Verkuil <hansverk@xxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: linux-fbdev@xxxxxxxxxxxxxxx
> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx>
> ---
> 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).
>
> 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
> + /** @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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
