Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1500128 > unrolled thread
| Started by | Brian Starkey <brian.starkey@arm.com> |
|---|---|
| First post | 2016-10-13 11:50 +0200 |
| Last post | 2016-10-17 08:30 +0200 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset Brian Starkey <brian.starkey@arm.com> - 2016-10-13 11:50 +0200
Re: [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset Alex Deucher <alexdeucher@gmail.com> - 2016-10-13 17:00 +0200
Re: [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset Daniel Vetter <daniel@ffwll.ch> - 2016-10-17 08:30 +0200
| From | Brian Starkey <brian.starkey@arm.com> |
|---|---|
| Date | 2016-10-13 11:50 +0200 |
| Subject | [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset |
| Message-ID | <srKOe-1Ic-11@gated-at.bofh.it> |
Add some additional comments to more explicitly describe the meaning and
usage of the three CRTC modeset detection booleans: mode_changed,
connectors_changed and active_changed.
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
Hi Daniel,
I guess I asked for this one :-), please just check my understanding
is correct.
Thanks,
Brian
drivers/gpu/drm/drm_atomic_helper.c | 9 +++++----
include/drm/drm_atomic.h | 11 ++++++++++-
include/drm/drm_crtc.h | 5 +++++
3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 78ea735..fb4071a 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -458,10 +458,11 @@ mode_fixup(struct drm_atomic_state *state)
* removed from the crtc.
* crtc_state->active_changed is set when crtc_state->active changes,
* which is used for dpms.
+ * See also: drm_atomic_crtc_needs_modeset()
*
* IMPORTANT:
*
- * Drivers which update ->mode_changed (e.g. in their ->atomic_check hooks if a
+ * Drivers which set ->mode_changed (e.g. in their ->atomic_check hooks if a
* plane update can't be done without a full modeset) _must_ call this function
* afterwards after that change. It is permitted to call this function multiple
* times for the same update, e.g. when the ->atomic_check functions depend upon
@@ -510,9 +511,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
for_each_connector_in_state(state, connector, connector_state, i) {
/*
- * This only sets crtc->mode_changed for routing changes,
- * drivers must set crtc->mode_changed themselves when connector
- * properties need to be updated.
+ * This only sets crtc->connectors_changed for routing changes,
+ * drivers must set crtc->connectors_changed themselves when
+ * connector properties need to be updated.
*/
ret = update_connector_routing(state, connector,
connector_state);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index d9aff06..1ce255f 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -368,8 +368,17 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state);
*
* To give drivers flexibility struct &drm_crtc_state has 3 booleans to track
* whether the state CRTC changed enough to need a full modeset cycle:
- * connectors_changed, mode_changed and active_change. This helper simply
+ * connectors_changed, mode_changed and active_changed. This helper simply
* combines these three to compute the overall need for a modeset for @state.
+ *
+ * The atomic helper code sets these booleans, but drivers can and should
+ * change them appropriately to accurately represent whether a modeset is
+ * really needed. In general, drivers should avoid full modesets whenever
+ * possible.
+ *
+ * For example if the CRTC mode has changed, and the hardware is able to enact
+ * the requested mode change without going through a full modeset, the driver
+ * should clear mode_changed during its ->atomic_check.
*/
static inline bool
drm_atomic_crtc_needs_modeset(struct drm_crtc_state *state)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c4a3164..1f094d2 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -116,6 +116,11 @@ struct drm_plane_helper_funcs;
* never return in a failure from the ->atomic_check callback. Userspace assumes
* that a DPMS On will always succeed. In other words: @enable controls resource
* assignment, @active controls the actual hardware state.
+ *
+ * The three booleans active_changed, connectors_changed and mode_changed are
+ * intended to indicate whether a full modeset is needed, rather than strictly
+ * describing what has changed in a commit.
+ * See also: drm_atomic_crtc_needs_modeset()
*/
struct drm_crtc_state {
struct drm_crtc *crtc;
--
1.7.9.5
[toc] | [next] | [standalone]
| From | Alex Deucher <alexdeucher@gmail.com> |
|---|---|
| Date | 2016-10-13 17:00 +0200 |
| Message-ID | <srPEe-4Qo-23@gated-at.bofh.it> |
| In reply to | #1500128 |
On Thu, Oct 13, 2016 at 5:47 AM, Brian Starkey <brian.starkey@arm.com> wrote:
> Add some additional comments to more explicitly describe the meaning and
> usage of the three CRTC modeset detection booleans: mode_changed,
> connectors_changed and active_changed.
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> ---
>
> Hi Daniel,
>
> I guess I asked for this one :-), please just check my understanding
> is correct.
>
The whole thread was very enlightening for me with respect to those
flags as well. The patch looks good to me.
Acked-by: Alex Deucher <alexander.deucher@amd.com>
> Thanks,
> Brian
>
> drivers/gpu/drm/drm_atomic_helper.c | 9 +++++----
> include/drm/drm_atomic.h | 11 ++++++++++-
> include/drm/drm_crtc.h | 5 +++++
> 3 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 78ea735..fb4071a 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -458,10 +458,11 @@ mode_fixup(struct drm_atomic_state *state)
> * removed from the crtc.
> * crtc_state->active_changed is set when crtc_state->active changes,
> * which is used for dpms.
> + * See also: drm_atomic_crtc_needs_modeset()
> *
> * IMPORTANT:
> *
> - * Drivers which update ->mode_changed (e.g. in their ->atomic_check hooks if a
> + * Drivers which set ->mode_changed (e.g. in their ->atomic_check hooks if a
> * plane update can't be done without a full modeset) _must_ call this function
> * afterwards after that change. It is permitted to call this function multiple
> * times for the same update, e.g. when the ->atomic_check functions depend upon
> @@ -510,9 +511,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>
> for_each_connector_in_state(state, connector, connector_state, i) {
> /*
> - * This only sets crtc->mode_changed for routing changes,
> - * drivers must set crtc->mode_changed themselves when connector
> - * properties need to be updated.
> + * This only sets crtc->connectors_changed for routing changes,
> + * drivers must set crtc->connectors_changed themselves when
> + * connector properties need to be updated.
> */
> ret = update_connector_routing(state, connector,
> connector_state);
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index d9aff06..1ce255f 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -368,8 +368,17 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state);
> *
> * To give drivers flexibility struct &drm_crtc_state has 3 booleans to track
> * whether the state CRTC changed enough to need a full modeset cycle:
> - * connectors_changed, mode_changed and active_change. This helper simply
> + * connectors_changed, mode_changed and active_changed. This helper simply
> * combines these three to compute the overall need for a modeset for @state.
> + *
> + * The atomic helper code sets these booleans, but drivers can and should
> + * change them appropriately to accurately represent whether a modeset is
> + * really needed. In general, drivers should avoid full modesets whenever
> + * possible.
> + *
> + * For example if the CRTC mode has changed, and the hardware is able to enact
> + * the requested mode change without going through a full modeset, the driver
> + * should clear mode_changed during its ->atomic_check.
> */
> static inline bool
> drm_atomic_crtc_needs_modeset(struct drm_crtc_state *state)
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c4a3164..1f094d2 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -116,6 +116,11 @@ struct drm_plane_helper_funcs;
> * never return in a failure from the ->atomic_check callback. Userspace assumes
> * that a DPMS On will always succeed. In other words: @enable controls resource
> * assignment, @active controls the actual hardware state.
> + *
> + * The three booleans active_changed, connectors_changed and mode_changed are
> + * intended to indicate whether a full modeset is needed, rather than strictly
> + * describing what has changed in a commit.
> + * See also: drm_atomic_crtc_needs_modeset()
> */
> struct drm_crtc_state {
> struct drm_crtc *crtc;
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[toc] | [prev] | [next] | [standalone]
| From | Daniel Vetter <daniel@ffwll.ch> |
|---|---|
| Date | 2016-10-17 08:30 +0200 |
| Subject | Re: [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset |
| Message-ID | <st9AS-8hu-13@gated-at.bofh.it> |
| In reply to | #1500319 |
On Thu, Oct 13, 2016 at 10:54:51AM -0400, Alex Deucher wrote:
> On Thu, Oct 13, 2016 at 5:47 AM, Brian Starkey <brian.starkey@arm.com> wrote:
> > Add some additional comments to more explicitly describe the meaning and
> > usage of the three CRTC modeset detection booleans: mode_changed,
> > connectors_changed and active_changed.
> >
> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> > ---
> >
> > Hi Daniel,
> >
> > I guess I asked for this one :-), please just check my understanding
> > is correct.
> >
>
> The whole thread was very enlightening for me with respect to those
> flags as well. The patch looks good to me.
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
Applied to drm-misc, thanks.
-Daniel
>
> > Thanks,
> > Brian
> >
> > drivers/gpu/drm/drm_atomic_helper.c | 9 +++++----
> > include/drm/drm_atomic.h | 11 ++++++++++-
> > include/drm/drm_crtc.h | 5 +++++
> > 3 files changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 78ea735..fb4071a 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -458,10 +458,11 @@ mode_fixup(struct drm_atomic_state *state)
> > * removed from the crtc.
> > * crtc_state->active_changed is set when crtc_state->active changes,
> > * which is used for dpms.
> > + * See also: drm_atomic_crtc_needs_modeset()
> > *
> > * IMPORTANT:
> > *
> > - * Drivers which update ->mode_changed (e.g. in their ->atomic_check hooks if a
> > + * Drivers which set ->mode_changed (e.g. in their ->atomic_check hooks if a
> > * plane update can't be done without a full modeset) _must_ call this function
> > * afterwards after that change. It is permitted to call this function multiple
> > * times for the same update, e.g. when the ->atomic_check functions depend upon
> > @@ -510,9 +511,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> >
> > for_each_connector_in_state(state, connector, connector_state, i) {
> > /*
> > - * This only sets crtc->mode_changed for routing changes,
> > - * drivers must set crtc->mode_changed themselves when connector
> > - * properties need to be updated.
> > + * This only sets crtc->connectors_changed for routing changes,
> > + * drivers must set crtc->connectors_changed themselves when
> > + * connector properties need to be updated.
> > */
> > ret = update_connector_routing(state, connector,
> > connector_state);
> > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> > index d9aff06..1ce255f 100644
> > --- a/include/drm/drm_atomic.h
> > +++ b/include/drm/drm_atomic.h
> > @@ -368,8 +368,17 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state);
> > *
> > * To give drivers flexibility struct &drm_crtc_state has 3 booleans to track
> > * whether the state CRTC changed enough to need a full modeset cycle:
> > - * connectors_changed, mode_changed and active_change. This helper simply
> > + * connectors_changed, mode_changed and active_changed. This helper simply
> > * combines these three to compute the overall need for a modeset for @state.
> > + *
> > + * The atomic helper code sets these booleans, but drivers can and should
> > + * change them appropriately to accurately represent whether a modeset is
> > + * really needed. In general, drivers should avoid full modesets whenever
> > + * possible.
> > + *
> > + * For example if the CRTC mode has changed, and the hardware is able to enact
> > + * the requested mode change without going through a full modeset, the driver
> > + * should clear mode_changed during its ->atomic_check.
> > */
> > static inline bool
> > drm_atomic_crtc_needs_modeset(struct drm_crtc_state *state)
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index c4a3164..1f094d2 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -116,6 +116,11 @@ struct drm_plane_helper_funcs;
> > * never return in a failure from the ->atomic_check callback. Userspace assumes
> > * that a DPMS On will always succeed. In other words: @enable controls resource
> > * assignment, @active controls the actual hardware state.
> > + *
> > + * The three booleans active_changed, connectors_changed and mode_changed are
> > + * intended to indicate whether a full modeset is needed, rather than strictly
> > + * describing what has changed in a commit.
> > + * See also: drm_atomic_crtc_needs_modeset()
> > */
> > struct drm_crtc_state {
> > struct drm_crtc *crtc;
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web