[PATCH] drm: Update docs for legacy kms state

[Daniel Vetter]

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 at intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> 
> 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 at intel.com>

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