Re: [PATCH 1/2] drm/doc: Document adjusted/request modes a bit better

From: Jose Abreu <Jose.Abreu@synopsys.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>,
	DRI Development <dri-devel@lists.freedesktop.org>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH 1/2] drm/doc: Document adjusted/request modes a bit better
Date: Tue, 16 May 2017 03:38:58 +0100	[thread overview]
Message-ID: <579860e9-0173-5d1a-a3f7-be812b59df04@synopsys.com> (raw)
In-Reply-To: <20170515091136.26307-1-daniel.vetter@ffwll.ch>

Hi Daniel,

On 15-05-2017 10:11, Daniel Vetter wrote:
> Laurent started a massive discussion on IRC about this. Let's try to
> document common usage a bit better.
>
> v2: Cross-links+typos.
>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jose Abreu <Jose.Abreu@synopsys.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Jose Abreu <joabreu@synopsys.com>

Best regards,
Jose Miguel Abreu

> ---
>  include/drm/drm_bridge.h                 |  2 +-
>  include/drm/drm_crtc.h                   | 28 +++++++++++++++++++++++++---
>  include/drm/drm_modeset_helper_vtables.h |  6 ++++--
>  3 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index f694de756ecf..f3ad38d0d621 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -91,7 +91,7 @@ struct drm_bridge_funcs {
>  	 * the display chain, either the final &drm_connector or the next
>  	 * &drm_bridge. The parameter adjusted_mode is the input mode the bridge
>  	 * requires. It can be modified by this callback and does not need to
> -	 * match mode.
> +	 * match mode. See also &drm_crtc_state.adjusted_mode for more details.
>  	 *
>  	 * This is the only hook that allows a bridge to reject a modeset. If
>  	 * this function passes all other callbacks must succeed for this
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 06236b002c22..5f5d53973ca5 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -90,8 +90,6 @@ struct drm_plane_helper_funcs;
>   * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
>   * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
>   * @encoder_mask: bitmask of (1 << drm_encoder_index(encoder)) of attached encoders
> - * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
> - * @mode: current mode timings
>   * @mode_blob: &drm_property_blob for @mode
>   * @state: backpointer to global drm_atomic_state
>   *
> @@ -131,9 +129,33 @@ struct drm_crtc_state {
>  	u32 connector_mask;
>  	u32 encoder_mask;
>  
> -	/* adjusted_mode: for use by helpers and drivers */
> +	/**
> +	 * @adjusted_mode:
> +	 *
> +	 * Internal display timings which can be used by the driver to handle
> +	 * differences between the mode requested by userspace in @mode and what
> +	 * is actually programmed into the hardware. It is purely driver
> +	 * implementation defined what exactly this adjusted mode means. Usually
> +	 * it is used to store the hardware display timings used between the
> +	 * CRTC and encoder blocks.
> +	 */
>  	struct drm_display_mode adjusted_mode;
>  
> +	/**
> +	 * @mode:
> +	 *
> +	 * Display timings requested by userspace. The driver should try to
> +	 * match the refresh rate as close as possible (but note that it's
> +	 * undefined what exactly is close enough, e.g. some of the HDMI modes
> +	 * only differ in less than 1% of the refresh rate). The active width
> +	 * and height as observed by userspace for positioning planes must match
> +	 * exactly.
> +	 *
> +	 * For external connectors where the sink isn't fixed (like with a
> +	 * built-in panel), this mode here should match the physical mode on the
> +	 * wire to the last details (i.e. including sync polarities and
> +	 * everything).
> +	 */
>  	struct drm_display_mode mode;
>  
>  	/* blob property to expose current mode to atomic userspace */
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 91d071ff1232..c72fca544a41 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -138,7 +138,8 @@ struct drm_crtc_helper_funcs {
>  	 * encoders need to be fed with. Note that this is the inverse semantics
>  	 * of the meaning for the &drm_encoder and &drm_bridge_funcs.mode_fixup
>  	 * vfunc. If the CRTC cannot support the requested conversion from mode
> -	 * to adjusted_mode it should reject the modeset.
> +	 * to adjusted_mode it should reject the modeset. See also
> +	 * &drm_crtc_state.adjusted_mode for more details.
>  	 *
>  	 * This function is used by both legacy CRTC helpers and atomic helpers.
>  	 * With atomic helpers it is optional.
> @@ -510,7 +511,8 @@ struct drm_encoder_helper_funcs {
>  	 * mode is the display mode that should be fed to the next element in
>  	 * the display chain, either the final &drm_connector or a &drm_bridge.
>  	 * The parameter adjusted_mode is the input mode the encoder requires. It
> -	 * can be modified by this callback and does not need to match mode.
> +	 * can be modified by this callback and does not need to match mode. See
> +	 * also &drm_crtc_state.adjusted_mode for more details.
>  	 *
>  	 * This function is used by both legacy CRTC helpers and atomic helpers.
>  	 * This hook is optional.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel