On Wed, Nov 08, 2017 at 09:30:07PM +0100, Daniel Vetter wrote:
> Point at the equivalent atomic state and explain that atomic drivers
> shouldn't really depend upon legacy state.
>
> Motivated by questions from Manasi about how this all is supposed to
> work.
>
> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Thanks for the patch, clarifies the usage of the atomic state
objects in place of legacy mode objects.
Reviewed-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 6 ++++++
> include/drm/drm_connector.h | 9 +++++++--
> include/drm/drm_encoder.h | 6 +++++-
> include/drm/drm_plane.h | 12 ++++++++++--
> 4 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ea781d55f2c1..09512a64c336 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -907,6 +907,12 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
> *
> * Drivers can use this for building their own atomic commit if they don't have
> * a pure helper-based modeset implementation.
> + *
> + * Since these updates are not synchronized with lockings, only code paths
> + * called from &drm_mode_config_helper_funcs.atomic_commit_tail can look at the
> + * legacy state filled out by this helper. Defacto this means this helper and
> + * the legacy state pointers are only really useful for transitioning an
> + * existing driver to the atomic world.
> */
> void
> drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index b4285c40e1e4..bb6629b5c55c 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -699,7 +699,6 @@ struct drm_cmdline_mode {
> * @force: a DRM_FORCE_<foo> state for forced mode sets
> * @override_edid: has the EDID been overwritten through debugfs for testing?
> * @encoder_ids: valid encoders for this connector
> - * @encoder: encoder driving this connector, if any
> * @eld: EDID-like data, if present
> * @latency_present: AV delay info from ELD, if found
> * @video_latency: video latency info from ELD, if found
> @@ -869,7 +868,13 @@ struct drm_connector {
>
> #define DRM_CONNECTOR_MAX_ENCODER 3
> uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER];
> - struct drm_encoder *encoder; /* currently active encoder */
> + /**
> + * @encoder: Currently bound encoder driving this connector, if any.
> + * Only really meaningful for non-atomic drivers. Atomic drivers should
> + * instead look at &drm_connector_state.best_encoder, and in case they
> + * need the CRTC driving this output, &drm_connector_state.crtc.
> + */
> + struct drm_encoder *encoder;
>
> #define MAX_ELD_BYTES 128
> /* EDID bits */
> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> index 86db0da8bdcb..67e1bccfb49d 100644
> --- a/include/drm/drm_encoder.h
> +++ b/include/drm/drm_encoder.h
> @@ -88,7 +88,6 @@ struct drm_encoder_funcs {
> * @head: list management
> * @base: base KMS object
> * @name: human readable name, can be overwritten by the driver
> - * @crtc: currently bound CRTC
> * @bridge: bridge associated to the encoder
> * @funcs: control functions
> * @helper_private: mid-layer private data
> @@ -166,6 +165,11 @@ struct drm_encoder {
> */
> uint32_t possible_clones;
>
> + /**
> + * @crtc: Currently bound CRTC, only really meaningful for non-atomic
> + * drivers. Atomic drivers should instead check
> + * &drm_connector_state.crtc.
> + */
> struct drm_crtc *crtc;
> struct drm_bridge *bridge;
> const struct drm_encoder_funcs *funcs;
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 069c4c8ce360..0efd38cd990b 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -474,8 +474,6 @@ enum drm_plane_type {
> * @format_types: array of formats supported by this plane
> * @format_count: number of formats supported
> * @format_default: driver hasn't supplied supported formats for the plane
> - * @crtc: currently bound CRTC
> - * @fb: currently bound fb
> * @old_fb: Temporary tracking of the old fb while a modeset is ongoing. Used by
> * drm_mode_set_config_internal() to implement correct refcounting.
> * @funcs: helper functions
> @@ -512,7 +510,17 @@ struct drm_plane {
> uint64_t *modifiers;
> unsigned int modifier_count;
>
> + /**
> + * @crtc: Currently bound CRTC, only really meaningful for non-atomic
> + * drivers. Atomic drivers should instead check &drm_plane_state.crtc.
> + */
> struct drm_crtc *crtc;
> +
> + /**
> + * @fb: Currently bound framebuffer, only really meaningful for
> + * non-atomic drivers. Atomic drivers should instead check
> + * &drm_plane_state.fb.
> + */
> struct drm_framebuffer *fb;
>
> struct drm_framebuffer *old_fb;
> --
> 2.15.0
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
