Re: [PATCH 05/17] drm: Add atomic driver interface definitions for objects

[Sean Paul <seanpaul@chromium.org>]

On Sun, Nov 02, 2014 at 02:19:18PM +0100, Daniel Vetter wrote:
> Heavily based upon Rob Clark's atomic series.
> - Dropped the connctor state from the crtc state, instead opting for a

nit: s/connctor/connector/

>   full-blown connector state. The only thing it has is the desired
>   crtc, but drivers which have connector properties have now a
>   data-structure to subclass.
>
> - Rename create_state to duplicate_state. Especially for legacy ioctls
>   we want updates on top of existing state, so we need a way to get at
>   the current state. We need to be careful to clear the backpointers
>   to the global state correctly though.
>
> - Drop property values. Drivers with properties simply need to
>   subclass the datastructures and track the decoded values in there. I
>   also think that common properties (like rotation) should be decoded
>   and stored in the core structures.
>
> - Create a new set of ->atomic_set_prop functions, for smoother
>   transitions from legacy to atomic operations.
>
> - Pass the ->atomic_set_prop ioctl the right structure to avoid
>   chasing pointers in drivers.
>
> - Drop temporary boolean state for now until we resurrect them with
>   the helper functions.
>
> - Drop invert_dimensions. For now we don't need any checking since
>   that's done by the higher-level legacy ioctls. But even then we
>   should also add rotation/flip tracking to the core drm_crtc_state,
>   not just whether the dimensions are inverted.
>
> - Track crtc state with an enable/disable. That's equivalent to
>   mode_valid, but a bit clearer that it means the entire crtc.
>
> The global interface will follow in subsequent patches.
>
> v2: We need to allow drivers to somehow set up the initial state and
> clear it on resume. So add a plane->reset callback for that. Helpers
> will be provided with default behaviour for all these.
>
> v3: Split out the plane->reset into a separate patch.
>
> v4: Improve kerneldoc in drm_crtc.h
>
> v5: Remove unused inline functions for handling state objects, those
> callbacks are now mandatory for full atomic support.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  include/drm/drm_crtc.h | 107 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a68e02be7e37..9847009ad451 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -224,6 +224,25 @@ struct drm_encoder;
>  struct drm_pending_vblank_event;
>  struct drm_plane;
>  struct drm_bridge;
> +struct drm_atomic_state;
> +
> +/**
> + * struct drm_crtc_state - mutable crtc state
> + * @enable: whether the CRTC should be enabled, gates all other state
> + * @mode: current mode timings
> + * @event: optional pointer to a DRM event to signal upon completion of the
> + * state update
> + * @state: backpointer to global drm_atomic_state
> + */
> +struct drm_crtc_state {
> + bool enable        : 1;
> +
> + struct drm_display_mode mode;
> +
> + struct drm_pending_vblank_event *event;
> +
> + struct drm_atomic_state *state;
> +};
>
>  /**
>   * struct drm_crtc_funcs - control CRTCs for a given device
> @@ -238,6 +257,9 @@ struct drm_bridge;
>   * @set_property: called when a property is changed
>   * @set_config: apply a new CRTC configuration
>   * @page_flip: initiate a page flip
> + * @atomic_duplicate_state: duplicate the atomic state for this CRTC
> + * @atomic_destroy_state: destroy an atomic state for this CRTC
> + * @atomic_set_property: set a property on an atomic state for this CRTC
>   *
>   * The drm_crtc_funcs structure is the central CRTC management structure
>   * in the DRM.  Each CRTC controls one or more connectors (note that the name
> @@ -288,6 +310,15 @@ struct drm_crtc_funcs {
>
>   int (*set_property)(struct drm_crtc *crtc,
>      struct drm_property *property, uint64_t val);
> +
> + /* atomic update handling */
> + struct drm_crtc_state *(*atomic_duplicate_state)(struct drm_crtc *crtc);
> + void (*atomic_destroy_state)(struct drm_crtc *crtc,
> +     struct drm_crtc_state *cstate);
> + int (*atomic_set_property)(struct drm_crtc *crtc,
> +   struct drm_crtc_state *state,
> +   struct drm_property *property,
> +   uint64_t val);
>  };
>
>  /**
> @@ -317,6 +348,7 @@ struct drm_crtc_funcs {
>   * @pixeldur_ns: precise pixel timing
>   * @helper_private: mid-layer private data
>   * @properties: property tracking for this CRTC
> + * @state: current atomic state for this CRTC
>   * @acquire_ctx: per-CRTC implicit acquire context used by atomic drivers for
>   * legacy ioctls
>   *
> @@ -374,6 +406,8 @@ struct drm_crtc {
>
>   struct drm_object_properties properties;
>
> + struct drm_crtc_state *state;
> +
>   /*
>   * For legacy crtc ioctls so that atomic drivers can get at the locking
>   * acquire context.
> @@ -381,6 +415,16 @@ struct drm_crtc {
>   struct drm_modeset_acquire_ctx *acquire_ctx;
>  };
>
> +/**
> + * struct drm_connector_state - mutable connector state
> + * @crtc: crtc to connect connector to, NULL if disabled
> + * @state: backpointer to global drm_atomic_state
> + */
> +struct drm_connector_state {
> + struct drm_crtc *crtc;
> +
> + struct drm_atomic_state *state;
> +};
>
>  /**
>   * struct drm_connector_funcs - control connectors on a given device
> @@ -393,6 +437,10 @@ struct drm_crtc {
>   * @set_property: property for this connector may need an update
>   * @destroy: make object go away
>   * @force: notify the driver that the connector is forced on
> + * @atomic_duplicate_state: duplicate the atomic state for this connector
> + * @atomic_destroy_state: destroy an atomic state for this connector
> + * @atomic_set_property: set a property on an atomic state for this connector
> + *
>   *
>   * Each CRTC may have one or more connectors attached to it.  The functions
>   * below allow the core DRM code to control connectors, enumerate available modes,
> @@ -417,6 +465,15 @@ struct drm_connector_funcs {
>       uint64_t val);
>   void (*destroy)(struct drm_connector *connector);
>   void (*force)(struct drm_connector *connector);
> +
> + /* atomic update handling */
> + struct drm_connector_state *(*atomic_duplicate_state)(struct drm_connector *connector);
> + void (*atomic_destroy_state)(struct drm_connector *connector,
> +     struct drm_connector_state *cstate);
> + int (*atomic_set_property)(struct drm_connector *connector,
> +   struct drm_connector_state *state,
> +   struct drm_property *property,
> +   uint64_t val);
>  };
>
>  /**
> @@ -515,6 +572,7 @@ struct drm_encoder {
>   * @null_edid_counter: track sinks that give us all zeros for the EDID
>   * @bad_edid_counter: track sinks that give us an EDID with invalid checksum
>   * @debugfs_entry: debugfs directory for this connector
> + * @state: current atomic state for this connector
>   *
>   * Each connector may be connected to one or more CRTCs, or may be clonable by
>   * another connector if they can share a CRTC.  Each connector also has a specific
> @@ -575,8 +633,42 @@ struct drm_connector {
>   unsigned bad_edid_counter;
>
>   struct dentry *debugfs_entry;
> +
> + struct drm_connector_state *state;
> +};
> +
> +/**
> + * struct drm_plane_state - mutable plane state
> + * @crtc: currently bound CRTC, NULL if disabled
> + * @fb: currently bound fb
> + * @crtc_x: left position of visible portion of plane on crtc
> + * @crtc_y: upper position of visible portion of plane on crtc
> + * @crtc_w: width of visible portion of plane on crtc
> + * @crtc_h: height of visible portion of plane on crtc
> + * @src_x: left position of visible portion of plane within
> + * plane (in 16.16)
> + * @src_y: upper position of visible portion of plane within
> + * plane (in 16.16)
> + * @src_w: width of visible portion of plane (in 16.16)
> + * @src_h: height of visible portion of plane (in 16.16)
> + * @state: backpointer to global drm_atomic_state
> + */
> +struct drm_plane_state {
> + struct drm_crtc *crtc;
> + struct drm_framebuffer *fb;
> +
> + /* Signed dest location allows it to be partially off screen */
> + int32_t crtc_x, crtc_y;
> + uint32_t crtc_w, crtc_h;
> +
> + /* Source values are 16.16 fixed point */
> + uint32_t src_x, src_y;
> + uint32_t src_h, src_w;
> +
> + struct drm_atomic_state *state;
>  };
>
> +
>  /**
>   * struct drm_plane_funcs - driver plane control functions
>   * @update_plane: update the plane configuration
> @@ -584,6 +676,9 @@ struct drm_connector {
>   * @destroy: clean up plane resources
>   * @reset: reset plane after state has been invalidated (e.g. resume)
>   * @set_property: called when a property is changed
> + * @atomic_duplicate_state: duplicate the atomic state for this plane
> + * @atomic_destroy_state: destroy an atomic state for this plane
> + * @atomic_set_property: set a property on an atomic state for this plane
>   */
>  struct drm_plane_funcs {
>   int (*update_plane)(struct drm_plane *plane,
> @@ -598,6 +693,15 @@ struct drm_plane_funcs {
>
>   int (*set_property)(struct drm_plane *plane,
>      struct drm_property *property, uint64_t val);
> +
> + /* atomic update handling */
> + struct drm_plane_state *(*atomic_duplicate_state)(struct drm_plane *plane);
> + void (*atomic_destroy_state)(struct drm_plane *plane,
> +     struct drm_plane_state *cstate);
> + int (*atomic_set_property)(struct drm_plane *plane,
> +   struct drm_plane_state *state,
> +   struct drm_property *property,
> +   uint64_t val);
>  };
>
>  enum drm_plane_type {
> @@ -621,6 +725,7 @@ enum drm_plane_type {
>   * @funcs: helper functions
>   * @properties: property tracking for this plane
>   * @type: type of plane (overlay, primary, cursor)
> + * @state: current atomic state for this plane
>   */
>  struct drm_plane {
>   struct drm_device *dev;
> @@ -642,6 +747,8 @@ struct drm_plane {
>   struct drm_object_properties properties;
>
>   enum drm_plane_type type;
> +
> + struct drm_plane_state *state;
>  };
>
>  /**
> --
> 2.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx