On Wed, Nov 08, 2017 at 12:57:30PM -0800, Manasi Navare wrote:
> 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>
Applied, thanks.
-Daniel
>
> > ---
> > 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
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
