LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Sean Paul <sean@poorly.run>
Cc: dri-devel@lists.freedesktop.org,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	Sean Paul <seanpaul@chromium.org>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	David Airlie <airlied@linux.ie>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 02/11] drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers
Date: Wed, 8 May 2019 18:33:27 +0200	[thread overview]
Message-ID: <20190508163327.GZ17751@phenom.ffwll.local> (raw)
In-Reply-To: <20190508160920.144739-3-sean@poorly.run>

On Wed, May 08, 2019 at 12:09:07PM -0400, Sean Paul wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Add functions to the atomic core to retrieve the old and new connectors
> associated with an encoder in a drm_atomic_state. This is useful for
> encoders and bridges that need to access the connector, for instance for
> the drm_display_info.
> 
> The CRTC associated with the encoder can also be retrieved through the
> connector state, and from it, the old and new CRTC states.
> 
> Changed in v4:
> - Added to the set
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> [seanpaul removed WARNs from helpers and added docs to explain why
> returning NULL might be valid]
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/drm_atomic.c | 70 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_atomic.h     |  7 ++++
>  2 files changed, 77 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5eb40130fafb..936002495523 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -797,6 +797,76 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
>  }
>  EXPORT_SYMBOL(drm_atomic_get_private_obj_state);
>  
> +/**
> + * drm_atomic_get_old_connector_for_encoder - Get old connector for an encoder
> + * @state: Atomic state
> + * @encoder: The encoder to fetch the connector state for
> + *
> + * This function finds and returns the connector that was connected to @encoder
> + * as specified by the @state.
> + *
> + * If there is no connector in @state which previously had @encoder connected to
> + * it, this function will return NULL. While this may seem like an invalid use
> + * case, it is sometimes a useful to differentiate commits which had no prior
> + * connectors attached to @encoder vs ones that did (and to inspect their
> + * @state). This is especially true in enable hooks because the pipeline has
> + * changed/will change.

s/has changed// ... I meant you'll pick the right ver tense :-)

> + *
> + * Returns: The old connector connected to @encoder, or NULL if the encoder is
> + * not connected.
> + */
> +struct drm_connector *
> +drm_atomic_get_old_connector_for_encoder(struct drm_atomic_state *state,
> +					 struct drm_encoder *encoder)
> +{
> +	struct drm_connector_state *conn_state;
> +	struct drm_connector *connector;
> +	unsigned int i;
> +
> +	for_each_old_connector_in_state(state, connector, conn_state, i) {
> +		if (conn_state->best_encoder == encoder)
> +			return connector;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL(drm_atomic_get_old_connector_for_encoder);
> +
> +/**
> + * drm_atomic_get_new_connector_for_encoder - Get new connector for an encoder
> + * @state: Atomic state
> + * @encoder: The encoder to fetch the connector state for
> + *
> + * This function finds and returns the connector that will be connected to
> + * @encoder as specified by the @state.
> + *
> + * If there is no connector in @state which will have @encoder connected to it,
> + * this function will return NULL. While this may seem like an invalid use case,
> + * it is sometimes a useful to differentiate commits which have no connectors
> + * attached to @encoder vs ones that do (and to inspect their state). This is
> + * especially true in disable hooks because the pipeline has changed/will
> + * change.

s/will change//

> + *
> + * Returns: The new connector connected to @encoder, or NULL if the encoder is
> + * not connected.
> + */
> +struct drm_connector *
> +drm_atomic_get_new_connector_for_encoder(struct drm_atomic_state *state,
> +					 struct drm_encoder *encoder)
> +{
> +	struct drm_connector_state *conn_state;
> +	struct drm_connector *connector;
> +	unsigned int i;
> +
> +	for_each_new_connector_in_state(state, connector, conn_state, i) {
> +		if (conn_state->best_encoder == encoder)
> +			return connector;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL(drm_atomic_get_new_connector_for_encoder);

Maybe also add a "See also drm_atomic_get_old_connector_for_encoder() and
drm_atomic_get_new_connector_for_encoder()." to the kerneldoc of
drm_connector_state.best_encoder.

With these doc nits:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +
>  /**
>   * drm_atomic_get_connector_state - get connector state
>   * @state: global atomic state object
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 824a5ed4e216..d6b3acd34e1c 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -453,6 +453,13 @@ struct drm_private_state * __must_check
>  drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
>  				 struct drm_private_obj *obj);
>  
> +struct drm_connector *
> +drm_atomic_get_old_connector_for_encoder(struct drm_atomic_state *state,
> +					 struct drm_encoder *encoder);
> +struct drm_connector *
> +drm_atomic_get_new_connector_for_encoder(struct drm_atomic_state *state,
> +					 struct drm_encoder *encoder);
> +
>  /**
>   * drm_atomic_get_existing_crtc_state - get crtc state, if it exists
>   * @state: global atomic state object
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

  reply	other threads:[~2019-05-08 16:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190508160920.144739-1-sean@poorly.run>
2019-05-08 16:09 ` [PATCH v4 01/11] drm: Add atomic variants of enable/disable to encoder helper funcs Sean Paul
2019-05-08 16:31   ` Daniel Vetter
2019-05-08 18:18     ` Sean Paul
2019-05-08 16:09 ` [PATCH v4 02/11] drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers Sean Paul
2019-05-08 16:33   ` Daniel Vetter [this message]
2019-05-12 16:48     ` Laurent Pinchart
2019-05-08 16:09 ` [PATCH v4 03/11] drm: Add atomic variants for bridge enable/disable Sean Paul
2019-05-21  8:58   ` Andrzej Hajda
2019-05-21 14:31     ` Sean Paul
2019-05-08 16:09 ` [PATCH v4 04/11] drm: Convert connector_helper_funcs->atomic_check to accept drm_atomic_state Sean Paul
2019-05-08 16:09 ` [PATCH v4 05/11] drm: Add helpers to kick off self refresh mode in drivers Sean Paul
2019-05-08 16:35   ` Daniel Vetter
2019-05-08 16:09 ` [PATCH v4 06/11] drm/rockchip: Use dirtyfb helper Sean Paul
2019-05-08 16:09 ` [PATCH v4 07/11] drm/rockchip: Check for fast link training before enabling psr Sean Paul
2019-05-08 16:09 ` [PATCH v4 08/11] drm/rockchip: Use the helpers for PSR Sean Paul
2019-05-08 16:09 ` [PATCH v4 09/11] drm/rockchip: Use vop_win in vop_win_disable instead of vop_win_data Sean Paul
2019-05-08 16:09 ` [PATCH v4 10/11] drm/rockchip: Don't fully disable vop on self refresh Sean Paul
2019-05-08 16:09 ` [PATCH v4 11/11] drm/rockchip: Use drm_atomic_helper_commit_tail_rpm Sean Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190508163327.GZ17751@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=maxime.ripard@bootlin.com \
    --cc=sean@poorly.run \
    --cc=seanpaul@chromium.org \
    --subject='Re: [PATCH v4 02/11] drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).