[Intel-gfx] [PATCH 15/17] drm/atomic-helpers: functions for state duplicate/destroy/reset
On Thu, Nov 06, 2014 at 12:43:43PM -0500, Sean Paul wrote: > On Sun, Nov 02, 2014 at 02:19:28PM +0100, Daniel Vetter wrote: > > The atomic users and helpers assume that there is always a obj->state > > structure around. Which means drivers need to somehow create that at > > driver load time. Also it should obviously reset hardware state, so > > needs to be reset upon resume. > > > > Finally the destroy/duplicate_state functions are an awful lot of > > boilerplate if the driver doesn't need anything beyond the default > > state objects. > > > > So add helper functions for all of this. > > > > v2: Somehow the plane/connector versions got lost in the first > > version. > > > > v3: Add kerneldoc. > > > > v4: Make duplicate_state functions a bit more robust, which is useful > > for debugging state tracking issues when transitioning to atomic. > > > > v5: Clear temporary variables in the crtc state when duplicating it, > > like ->mode_changed or ->planes_changed. If we don't do this stale > > values for these might pollute the next atomic modeset. > > > > v6: Also clear crtc_state->event in case the driver didn't (yet) clear > > this out. > > > > Signed-off-by: Daniel Vetter > > Minus the fixup nits that danielt pointed out Yeah already fixed them but forgotten to resend the patch. Done now. > > Reviewed-by: Sean Paul Can you please double check that the new version is ok? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Intel-gfx] [PATCH 15/17] drm/atomic-helpers: functions for state duplicate/destroy/reset
On Thu, Nov 6, 2014 at 2:57 PM, Daniel Vetter wrote: > On Thu, Nov 06, 2014 at 12:43:43PM -0500, Sean Paul wrote: >> On Sun, Nov 02, 2014 at 02:19:28PM +0100, Daniel Vetter wrote: >> > The atomic users and helpers assume that there is always a obj->state >> > structure around. Which means drivers need to somehow create that at >> > driver load time. Also it should obviously reset hardware state, so >> > needs to be reset upon resume. >> > >> > Finally the destroy/duplicate_state functions are an awful lot of >> > boilerplate if the driver doesn't need anything beyond the default >> > state objects. >> > >> > So add helper functions for all of this. >> > >> > v2: Somehow the plane/connector versions got lost in the first >> > version. >> > >> > v3: Add kerneldoc. >> > >> > v4: Make duplicate_state functions a bit more robust, which is useful >> > for debugging state tracking issues when transitioning to atomic. >> > >> > v5: Clear temporary variables in the crtc state when duplicating it, >> > like ->mode_changed or ->planes_changed. If we don't do this stale >> > values for these might pollute the next atomic modeset. >> > >> > v6: Also clear crtc_state->event in case the driver didn't (yet) clear >> > this out. >> > >> > Signed-off-by: Daniel Vetter >> >> Minus the fixup nits that danielt pointed out > > Yeah already fixed them but forgotten to resend the patch. Done now. >> >> Reviewed-by: Sean Paul > > Can you please double check that the new version is ok? > Reviewed-by: Sean Paul > Thanks, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Intel-gfx] [PATCH 15/17] drm/atomic-helpers: functions for state duplicate/destroy/reset
On Sun, Nov 02, 2014 at 02:19:28PM +0100, Daniel Vetter wrote:
> The atomic users and helpers assume that there is always a obj->state
> structure around. Which means drivers need to somehow create that at
> driver load time. Also it should obviously reset hardware state, so
> needs to be reset upon resume.
>
> Finally the destroy/duplicate_state functions are an awful lot of
> boilerplate if the driver doesn't need anything beyond the default
> state objects.
>
> So add helper functions for all of this.
>
> v2: Somehow the plane/connector versions got lost in the first
> version.
>
> v3: Add kerneldoc.
>
> v4: Make duplicate_state functions a bit more robust, which is useful
> for debugging state tracking issues when transitioning to atomic.
>
> v5: Clear temporary variables in the crtc state when duplicating it,
> like ->mode_changed or ->planes_changed. If we don't do this stale
> values for these might pollute the next atomic modeset.
>
> v6: Also clear crtc_state->event in case the driver didn't (yet) clear
> this out.
>
> Signed-off-by: Daniel Vetter
Minus the fixup nits that danielt pointed out
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 154
> +++-
> include/drm/drm_atomic_helper.h | 19 +
> 2 files changed, 170 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 70bd67cf86e3..bd38df3cbe55 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1429,7 +1429,7 @@ EXPORT_SYMBOL(drm_atomic_helper_set_config);
> /**
> * drm_atomic_helper_crtc_set_property - helper for crtc prorties
> * @crtc: DRM crtc
> - * @prorty: DRM property
> + * @property: DRM property
> * @val: value of property
> *
> * Provides a default plane disablle handler using the atomic driver
> interface.
> @@ -1493,7 +1493,7 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_set_property);
> /**
> * drm_atomic_helper_plane_set_property - helper for plane prorties
> * @plane: DRM plane
> - * @prorty: DRM property
> + * @property: DRM property
> * @val: value of property
> *
> * Provides a default plane disable handler using the atomic driver
> interface.
> @@ -1557,7 +1557,7 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_set_property);
> /**
> * drm_atomic_helper_connector_set_property - helper for connector prorties
> * @connector: DRM connector
> - * @prorty: DRM property
> + * @property: DRM property
> * @val: value of property
> *
> * Provides a default plane disablle handler using the atomic driver
> interface.
> @@ -1707,3 +1707,151 @@ backoff:
> goto retry;
> }
> EXPORT_SYMBOL(drm_atomic_helper_page_flip);
> +
> +/**
> + * drm_atomic_helper_crtc_reset - default ->reset hook for CRTCs
> + * @crtc: drm CRTC
> + *
> + * Resets the atomic state for @crtc by freeing the state pointer and
> allocating
> + * a new empty state object.
> + */
> +void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
> +{
> + kfree(crtc->state);
> + crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
> +
> +/**
> + * drm_atomic_helper_crtc_duplicate_state - default state duplicate hook
> + * @crtc: drm CRTC
> + *
> + * Default CRTC state duplicate hook for drivers which don't have their own
> + * subclassed CRTC state structure.
> + */
> +struct drm_crtc_state *
> +drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc)
> +{
> + struct drm_crtc_state *state;
> +
> + if (WARN_ON(!crtc->state))
> + return NULL;
> +
> + state = kmemdup(crtc->state, sizeof(*crtc->state), GFP_KERNEL);
> +
> + if (state) {
> + state->mode_changed = false;
> + state->planes_changed = false;
> + state->event = NULL;
> + }
> +
> + return state;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
> +
> +/**
> + * drm_atomic_helper_crtc_destroy_state - default state destroy hook
> + * @crtc: drm CRTC
> + * @state: CRTC state object to release
> + *
> + * Default CRTC state destroy hook for drivers which don't have their own
> + * subclassed CRTC state structure.
> + */
> +void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> + struct drm_crtc_state *state)
> +{
> + kfree(state);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
> +
> +/**
> + * drm_atomic_helper_plane_reset - default ->reset hook for planes
> + * @plane: drm plane
> + *
> + * Resets the atomic state for @plane by freeing the state pointer and
> + * allocating a new empty state object.
> + */
> +void drm_atomic_helper_plane_reset(struct drm_plane *plane)
> +{
> + kfree(plane->state);
> + plane->state = kzalloc(sizeof(*plane->state), GFP_KERNEL);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
> +
> +/**
> + * drm_atomic_helper_plane_duplicate_state - default state duplicate hook
> + * @plane: drm plane
> + *
> + * Default plane state duplicate hook for drivers which don't have their own
> + *
[PATCH 15/17] drm/atomic-helpers: functions for state duplicate/destroy/reset
On Mon, Nov 03, 2014 at 03:06:07PM +, Daniel Thompson wrote: > > kfree is a nop when the argument is NULL, which is a crucial property of > > this - memset would oops on driver load. > > Oops. Missed that (I think I misread who as assuming there was always > obj->state in the patch header). > > Do you fancy making the comment "by freeing the state pointer and > allocating a new..." into "by freeing the state pointer (which may be > NULL) and allocating a new...". > > If nothing else that means the documentation is richer than the code... Yeah, sounds like a good idea. I'll also mention that this is for driver load mostly (if the driver doesn't decide to throw away the state for some reason over suspend/resume). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 15/17] drm/atomic-helpers: functions for state duplicate/destroy/reset
On Mon, Nov 03, 2014 at 02:45:28PM +, Daniel Thompson wrote:
> > index 70bd67cf86e3..bd38df3cbe55 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1429,7 +1429,7 @@ EXPORT_SYMBOL(drm_atomic_helper_set_config);
> > /**
> > * drm_atomic_helper_crtc_set_property - helper for crtc prorties
> > * @crtc: DRM crtc
> > - * @prorty: DRM property
> > + * @property: DRM property
>
> This looks like a bad fixup (should be in patch 11).
Indeed, will shuffle around.
> > +void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
> > +{
> > + kfree(crtc->state);
> > + crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
>
> This code looks semantically equivalent to a memset() although it may
> result in a change to the pointer value. Is this code trying to flush
> out uses-after-free?
>
> I can't find this free/alloc pattern in delivered code anywhere else in
> the drm code base. Should this need to be replaced with memset() before
> merging (or at least commenting)?
kfree is a nop when the argument is NULL, which is a crucial property of
this - memset would oops on driver load.
Even neglecting this a memset imo doesn't blow up loudly enough if the
driver subclasses the state structs (by adding more of it's driver private
state at the end). Whereas underallocating tends to anger the slab
poisoning code badly.
Finally it's really not just a memset, but a free + realloc. See the plane
state, which also needs to drop a potential fb reference. Imo the explicit
kfree+realloc makes that more obvious.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 15/17] drm/atomic-helpers: functions for state duplicate/destroy/reset
On 03/11/14 14:53, Daniel Vetter wrote:
> On Mon, Nov 03, 2014 at 02:45:28PM +, Daniel Thompson wrote:
>>> index 70bd67cf86e3..bd38df3cbe55 100644
>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>> @@ -1429,7 +1429,7 @@ EXPORT_SYMBOL(drm_atomic_helper_set_config);
>>> /**
>>> * drm_atomic_helper_crtc_set_property - helper for crtc prorties
>>> * @crtc: DRM crtc
>>> - * @prorty: DRM property
>>> + * @property: DRM property
>>
>> This looks like a bad fixup (should be in patch 11).
>
> Indeed, will shuffle around.
>
>>> +void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
>>> +{
>>> + kfree(crtc->state);
>>> + crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
>>
>> This code looks semantically equivalent to a memset() although it may
>> result in a change to the pointer value. Is this code trying to flush
>> out uses-after-free?
>>
>> I can't find this free/alloc pattern in delivered code anywhere else in
>> the drm code base. Should this need to be replaced with memset() before
>> merging (or at least commenting)?
>
> kfree is a nop when the argument is NULL, which is a crucial property of
> this - memset would oops on driver load.
Oops. Missed that (I think I misread who as assuming there was always
obj->state in the patch header).
Do you fancy making the comment "by freeing the state pointer and
allocating a new..." into "by freeing the state pointer (which may be
NULL) and allocating a new...".
If nothing else that means the documentation is richer than the code...
> Even neglecting this a memset imo doesn't blow up loudly enough if the
> driver subclasses the state structs (by adding more of it's driver private
> state at the end). Whereas underallocating tends to anger the slab
> poisoning code badly.
>
> Finally it's really not just a memset, but a free + realloc. See the plane
> state, which also needs to drop a potential fb reference. Imo the explicit
> kfree+realloc makes that more obvious.
[PATCH 15/17] drm/atomic-helpers: functions for state duplicate/destroy/reset
On 02/11/14 13:19, Daniel Vetter wrote:> The atomic users and helpers
assume that there is always a obj->state
> structure around. Which means drivers need to somehow create that at
> driver load time. Also it should obviously reset hardware state, so
> needs to be reset upon resume.
>
> Finally the destroy/duplicate_state functions are an awful lot of
> boilerplate if the driver doesn't need anything beyond the default
> state objects.
>
> So add helper functions for all of this.
>
> v2: Somehow the plane/connector versions got lost in the first
> version.
>
> v3: Add kerneldoc.
>
> v4: Make duplicate_state functions a bit more robust, which is useful
> for debugging state tracking issues when transitioning to atomic.
>
> v5: Clear temporary variables in the crtc state when duplicating it,
> like ->mode_changed or ->planes_changed. If we don't do this stale
> values for these might pollute the next atomic modeset.
>
> v6: Also clear crtc_state->event in case the driver didn't (yet) clear
> this out.
>
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 154
+++-
> include/drm/drm_atomic_helper.h | 19 +
> 2 files changed, 170 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
> index 70bd67cf86e3..bd38df3cbe55 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1429,7 +1429,7 @@ EXPORT_SYMBOL(drm_atomic_helper_set_config);
> /**
> * drm_atomic_helper_crtc_set_property - helper for crtc prorties
> * @crtc: DRM crtc
> - * @prorty: DRM property
> + * @property: DRM property
This looks like a bad fixup (should be in patch 11).
> * @val: value of property
> *
> * Provides a default plane disablle handler using the atomic driver
interface.
> @@ -1493,7 +1493,7 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_set_property);
> /**
> * drm_atomic_helper_plane_set_property - helper for plane prorties
> * @plane: DRM plane
> - * @prorty: DRM property
> + * @property: DRM property
+1
> * @val: value of property
> *
> * Provides a default plane disable handler using the atomic driver
interface.
> @@ -1557,7 +1557,7 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_set_property);
> /**
> * drm_atomic_helper_connector_set_property - helper for connector
prorties
> * @connector: DRM connector
> - * @prorty: DRM property
> + * @property: DRM property
+1
> * @val: value of property
> *
> * Provides a default plane disablle handler using the atomic driver
interface.
> @@ -1707,3 +1707,151 @@ backoff:
> goto retry;
> }
> EXPORT_SYMBOL(drm_atomic_helper_page_flip);
> +
> +/**
> + * drm_atomic_helper_crtc_reset - default ->reset hook for CRTCs
> + * @crtc: drm CRTC
> + *
> + * Resets the atomic state for @crtc by freeing the state pointer and
allocating
> + * a new empty state object.
> + */
> +void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
> +{
> + kfree(crtc->state);
> + crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
This code looks semantically equivalent to a memset() although it may
result in a change to the pointer value. Is this code trying to flush
out uses-after-free?
I can't find this free/alloc pattern in delivered code anywhere else in
the drm code base. Should this need to be replaced with memset() before
merging (or at least commenting)?
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
> +
> +/**
> + * drm_atomic_helper_crtc_duplicate_state - default state duplicate hook
> + * @crtc: drm CRTC
> + *
> + * Default CRTC state duplicate hook for drivers which don't have
their own
> + * subclassed CRTC state structure.
> + */
> +struct drm_crtc_state *
> +drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc)
> +{
> + struct drm_crtc_state *state;
> +
> + if (WARN_ON(!crtc->state))
> + return NULL;
> +
> + state = kmemdup(crtc->state, sizeof(*crtc->state), GFP_KERNEL);
> +
> + if (state) {
> + state->mode_changed = false;
> + state->planes_changed = false;
> + state->event = NULL;
> + }
> +
> + return state;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
> +
> +/**
> + * drm_atomic_helper_crtc_destroy_state - default state destroy hook
> + * @crtc: drm CRTC
> + * @state: CRTC state object to release
> + *
> + * Default CRTC state destroy hook for drivers which don't have their own
> + * subclassed CRTC state structure.
> + */
> +void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> + struct drm_crtc_state *state)
> +{
> + kfree(state);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
> +
> +/**
> + * drm_atomic_helper_plane_reset - default ->reset hook for planes
> + * @plane: drm plane
> + *
> + * Resets the atomic state for @plane by freeing the state pointer and
> + * allocating a new empty state object.
[PATCH 15/17] drm/atomic-helpers: functions for state duplicate/destroy/reset
The atomic users and helpers assume that there is always a obj->state
structure around. Which means drivers need to somehow create that at
driver load time. Also it should obviously reset hardware state, so
needs to be reset upon resume.
Finally the destroy/duplicate_state functions are an awful lot of
boilerplate if the driver doesn't need anything beyond the default
state objects.
So add helper functions for all of this.
v2: Somehow the plane/connector versions got lost in the first
version.
v3: Add kerneldoc.
v4: Make duplicate_state functions a bit more robust, which is useful
for debugging state tracking issues when transitioning to atomic.
v5: Clear temporary variables in the crtc state when duplicating it,
like ->mode_changed or ->planes_changed. If we don't do this stale
values for these might pollute the next atomic modeset.
v6: Also clear crtc_state->event in case the driver didn't (yet) clear
this out.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_atomic_helper.c | 154 +++-
include/drm/drm_atomic_helper.h | 19 +
2 files changed, 170 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 70bd67cf86e3..bd38df3cbe55 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1429,7 +1429,7 @@ EXPORT_SYMBOL(drm_atomic_helper_set_config);
/**
* drm_atomic_helper_crtc_set_property - helper for crtc prorties
* @crtc: DRM crtc
- * @prorty: DRM property
+ * @property: DRM property
* @val: value of property
*
* Provides a default plane disablle handler using the atomic driver interface.
@@ -1493,7 +1493,7 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_set_property);
/**
* drm_atomic_helper_plane_set_property - helper for plane prorties
* @plane: DRM plane
- * @prorty: DRM property
+ * @property: DRM property
* @val: value of property
*
* Provides a default plane disable handler using the atomic driver interface.
@@ -1557,7 +1557,7 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_set_property);
/**
* drm_atomic_helper_connector_set_property - helper for connector prorties
* @connector: DRM connector
- * @prorty: DRM property
+ * @property: DRM property
* @val: value of property
*
* Provides a default plane disablle handler using the atomic driver interface.
@@ -1707,3 +1707,151 @@ backoff:
goto retry;
}
EXPORT_SYMBOL(drm_atomic_helper_page_flip);
+
+/**
+ * drm_atomic_helper_crtc_reset - default ->reset hook for CRTCs
+ * @crtc: drm CRTC
+ *
+ * Resets the atomic state for @crtc by freeing the state pointer and
allocating
+ * a new empty state object.
+ */
+void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
+{
+ kfree(crtc->state);
+ crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
+}
+EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
+
+/**
+ * drm_atomic_helper_crtc_duplicate_state - default state duplicate hook
+ * @crtc: drm CRTC
+ *
+ * Default CRTC state duplicate hook for drivers which don't have their own
+ * subclassed CRTC state structure.
+ */
+struct drm_crtc_state *
+drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc)
+{
+ struct drm_crtc_state *state;
+
+ if (WARN_ON(!crtc->state))
+ return NULL;
+
+ state = kmemdup(crtc->state, sizeof(*crtc->state), GFP_KERNEL);
+
+ if (state) {
+ state->mode_changed = false;
+ state->planes_changed = false;
+ state->event = NULL;
+ }
+
+ return state;
+}
+EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
+
+/**
+ * drm_atomic_helper_crtc_destroy_state - default state destroy hook
+ * @crtc: drm CRTC
+ * @state: CRTC state object to release
+ *
+ * Default CRTC state destroy hook for drivers which don't have their own
+ * subclassed CRTC state structure.
+ */
+void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
+ struct drm_crtc_state *state)
+{
+ kfree(state);
+}
+EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
+
+/**
+ * drm_atomic_helper_plane_reset - default ->reset hook for planes
+ * @plane: drm plane
+ *
+ * Resets the atomic state for @plane by freeing the state pointer and
+ * allocating a new empty state object.
+ */
+void drm_atomic_helper_plane_reset(struct drm_plane *plane)
+{
+ kfree(plane->state);
+ plane->state = kzalloc(sizeof(*plane->state), GFP_KERNEL);
+}
+EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
+
+/**
+ * drm_atomic_helper_plane_duplicate_state - default state duplicate hook
+ * @plane: drm plane
+ *
+ * Default plane state duplicate hook for drivers which don't have their own
+ * subclassed plane state structure.
+ */
+struct drm_plane_state *
+drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane)
+{
+ if (WARN_ON(!plane->state))
+ return NULL;
+
+ return kmemdup(plane->state, sizeof(*plane->state),
