Re: [PATCH v4 02/11] drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers
Hi Sean,
In the subject line, s/-/)/
On Wed, May 08, 2019 at 06:33:27PM +0200, Daniel Vetter wrote:
> On Wed, May 08, 2019 at 12:09:07PM -0400, Sean Paul wrote:
> > From: Laurent Pinchart
> >
> > 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
> > Signed-off-by: Laurent Pinchart
> >
> > [seanpaul removed WARNs from helpers and added docs to explain why
> > returning NULL might be valid]
> > Signed-off-by: Sean Paul
> > ---
> > 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
s/a useful/useful/
> > + * connectors attached to @encoder vs ones that did (and to inspect their
> > + * @state). This is especially true in enable hooks because the pipeline
> > has
s/@state/state/, otherwise you refer to the parameter passed to this
function, which is not a connector state.
> > + * 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
s/a useful/useful/
> > + * 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
>
> > +
> > /**
> > * drm_atomic_get_connector_state - get connector state
> > * @state: global atomic state object
>
Re: [PATCH v4 02/11] drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers
On Wed, May 08, 2019 at 12:09:07PM -0400, Sean Paul wrote:
> From: Laurent Pinchart
>
> 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
> Signed-off-by: Laurent Pinchart
>
> [seanpaul removed WARNs from helpers and added docs to explain why
> returning NULL might be valid]
> Signed-off-by: Sean Paul
> ---
> 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
> +
> /**
> * 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,
> +
[PATCH v4 02/11] drm: Add drm_atomic_get_(old|new-_connector_for_encoder() helpers
From: Laurent Pinchart
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
Signed-off-by: Laurent Pinchart
[seanpaul removed WARNs from helpers and added docs to explain why
returning NULL might be valid]
Signed-off-by: Sean Paul
---
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.
+ *
+ * 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.
+ *
+ * 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);
+
/**
* 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
