Re: [RFC v3 05/33] rust: drm/kms: Add drm_plane bindings
From: Maxime Ripard
Date: Fri Mar 14 2025 - 07:39:23 EST
On Wed, Mar 05, 2025 at 05:59:21PM -0500, Lyude Paul wrote:
> The next step is adding a set of basic bindings to create a plane, which
> has to happen before we can create a CRTC (since we need to be able to at
> least specify a primary plane for a CRTC upon creation). This mostly
> follows the same general pattern as connectors (AsRawPlane,
> AsRawPlaneState, etc.).
>
> There is one major difference with planes vs. other types of atomic mode
> objects: drm_plane_state isn't the only base plane struct used in DRM
> drivers, as some drivers will use helpers like drm_shadow_plane_state which
> have a drm_plane_state embedded within them.
It's one of the things where I also think you could have broken down the
patch some more. drm_shadow_plane_state, while useful, is not central
and could be handled later on. Both would ease the review, and we could
merge the "central" plane support without it if we disagree only on that
part for example.
> Since we'll eventually be adding bindings for shadow planes, we introduce a
> PlaneStateHelper trait - which represents any data type which can be used
> as the main wrapping structure around a drm_plane_state - and we implement
> this trait for PlaneState<T>. This trait can be used in our C callbacks to
> allow for drivers to use different wrapping structures without needing to
> implement a separate set of FFI callbacks for each type. Currently planes
> are the only type I'm aware of which do this.
And I don't think it's any different to any other driver structure. It
looks like most of the users except two don't have any additional data
so can't we do something like
struct ShadowPlaneState<T: Default> {
...,
data: T,
}
And just put that into PlaneState just like any other driver?
Maxime
Attachment:
signature.asc
Description: PGP signature