* [RFC] drm: Helper macro for drm state duplication
@ 2017-02-09 15:41 ` Mihail Atanassov
0 siblings, 0 replies; 12+ messages in thread
From: Mihail Atanassov @ 2017-02-09 15:41 UTC (permalink / raw)
To: dri-devel
Cc: Daniel Vetter, Jani Nikula, Sean Paul, David Airlie, Liviu Dudau,
Brian Starkey, linux-kernel, nd
Hi,
I was working on a few patches adding fields to struct malidp_crtc_state and
found myself writing memcpy multiple times in the ->atomic_duplicate_state
hook because I wanted to avoid copying the drm_crtc_state twice
(__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
also applies to the other drm_*_state derivatives, so I concocted a macro
helper to do the copy in one chunk (two if you count the __drm_atomic_helper*
one). I'd appreciate some comments on whether anyone else might find this
macro useful. Thanks!
Mihail Atanassov (1):
drm: Add helper macro for duplicating custom drm_*_state
include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
--
Mihail Atanassov
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC] drm: Helper macro for drm state duplication
@ 2017-02-09 15:41 ` Mihail Atanassov
0 siblings, 0 replies; 12+ messages in thread
From: Mihail Atanassov @ 2017-02-09 15:41 UTC (permalink / raw)
To: dri-devel; +Cc: Liviu Dudau, linux-kernel, Daniel Vetter, nd
Hi,
I was working on a few patches adding fields to struct malidp_crtc_state and
found myself writing memcpy multiple times in the ->atomic_duplicate_state
hook because I wanted to avoid copying the drm_crtc_state twice
(__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
also applies to the other drm_*_state derivatives, so I concocted a macro
helper to do the copy in one chunk (two if you count the __drm_atomic_helper*
one). I'd appreciate some comments on whether anyone else might find this
macro useful. Thanks!
Mihail Atanassov (1):
drm: Add helper macro for duplicating custom drm_*_state
include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
--
Mihail Atanassov
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] drm: Add helper macro for duplicating custom drm_*_state
2017-02-09 15:41 ` Mihail Atanassov
@ 2017-02-09 15:41 ` Mihail Atanassov
-1 siblings, 0 replies; 12+ messages in thread
From: Mihail Atanassov @ 2017-02-09 15:41 UTC (permalink / raw)
To: dri-devel
Cc: Daniel Vetter, Jani Nikula, Sean Paul, David Airlie, Liviu Dudau,
Brian Starkey, linux-kernel, nd
Assuming a derived struct of the form:
struct foo_bar_state
{
struct drm_bar_state bar_state;
struct foo_private priv;
struct foo_private2 *priv2;
};
memcpy priv and priv2 to the new instance of foo_bar_state. The
intention is to use this macro in ->atomic_duplicate_state in conjunction with
__drm_atomic_helper_*_duplicate_state, which already copies the relevant
drm_*_state struct.
There's an equality check for new_state and old_state to ensure that
they are distinct instances of the same type, and a BUILD_BUG if the
base struct (bar_state in the above example) is not first in the derived
struct, to avoid missing any data before it and corrupting the base's data.
Signed-off-by: Mihail Atanassov <mihail.atanassov@arm.com>
---
include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index d066e94..ecc6a82 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -171,6 +171,39 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
uint32_t size);
/**
+ * drm_atomic_duplicate_custom_state - helper macro for duplicating
+ * driver-private additions to drm_*_state.
+ * @new_state: pointer to destination state struct
+ * @old_state: pointer to source state struct
+ * @basename: name of the drm_*_state member of the new_state/old_state struct
+ *
+ * Copies the data after the base struct until the end of the custom struct,
+ * e.g. given a structure
+ *
+ * struct foo_bar_state {
+ * struct drm_bar_state base;
+ * struct foo_private priv;
+ * struct foo_private2 *priv2;
+ * };
+ *
+ * this copies priv and priv2. NB: the base struct *must* be the first element
+ * of the derived struct, and new_state and old_state have to be two distinct
+ * instances.
+ */
+#define drm_atomic_helper_duplicate_custom_state(new_state, old_state, basename) \
+ do { \
+ size_t base_size = sizeof(new_state->basename); \
+ size_t base_offset = offsetof(typeof(*new_state), basename); \
+ \
+ BUILD_BUG_ON(base_offset != 0); \
+ if (new_state == old_state) /* Type-check */ \
+ break; \
+ memcpy((char *)new_state + base_size, \
+ (char *)old_state + base_size, \
+ sizeof(*new_state) - base_size); \
+ } while(0)
+
+/**
* drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
* @plane: the loop cursor
* @crtc: the crtc whose planes are iterated
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] drm: Add helper macro for duplicating custom drm_*_state
@ 2017-02-09 15:41 ` Mihail Atanassov
0 siblings, 0 replies; 12+ messages in thread
From: Mihail Atanassov @ 2017-02-09 15:41 UTC (permalink / raw)
To: dri-devel
Cc: Daniel Vetter, Jani Nikula, Sean Paul, David Airlie, Liviu Dudau,
Brian Starkey, linux-kernel, nd
Assuming a derived struct of the form:
struct foo_bar_state
{
struct drm_bar_state bar_state;
struct foo_private priv;
struct foo_private2 *priv2;
};
memcpy priv and priv2 to the new instance of foo_bar_state. The
intention is to use this macro in ->atomic_duplicate_state in conjunction with
__drm_atomic_helper_*_duplicate_state, which already copies the relevant
drm_*_state struct.
There's an equality check for new_state and old_state to ensure that
they are distinct instances of the same type, and a BUILD_BUG if the
base struct (bar_state in the above example) is not first in the derived
struct, to avoid missing any data before it and corrupting the base's data.
Signed-off-by: Mihail Atanassov <mihail.atanassov@arm.com>
---
include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index d066e94..ecc6a82 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -171,6 +171,39 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
uint32_t size);
/**
+ * drm_atomic_duplicate_custom_state - helper macro for duplicating
+ * driver-private additions to drm_*_state.
+ * @new_state: pointer to destination state struct
+ * @old_state: pointer to source state struct
+ * @basename: name of the drm_*_state member of the new_state/old_state struct
+ *
+ * Copies the data after the base struct until the end of the custom struct,
+ * e.g. given a structure
+ *
+ * struct foo_bar_state {
+ * struct drm_bar_state base;
+ * struct foo_private priv;
+ * struct foo_private2 *priv2;
+ * };
+ *
+ * this copies priv and priv2. NB: the base struct *must* be the first element
+ * of the derived struct, and new_state and old_state have to be two distinct
+ * instances.
+ */
+#define drm_atomic_helper_duplicate_custom_state(new_state, old_state, basename) \
+ do { \
+ size_t base_size = sizeof(new_state->basename); \
+ size_t base_offset = offsetof(typeof(*new_state), basename); \
+ \
+ BUILD_BUG_ON(base_offset != 0); \
+ if (new_state == old_state) /* Type-check */ \
+ break; \
+ memcpy((char *)new_state + base_size, \
+ (char *)old_state + base_size, \
+ sizeof(*new_state) - base_size); \
+ } while(0)
+
+/**
* drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
* @plane: the loop cursor
* @crtc: the crtc whose planes are iterated
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC] drm: Helper macro for drm state duplication
2017-02-09 15:41 ` Mihail Atanassov
@ 2017-02-09 16:57 ` Daniel Vetter
-1 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-02-09 16:57 UTC (permalink / raw)
To: Mihail Atanassov; +Cc: dri-devel, Liviu Dudau, linux-kernel, Daniel Vetter, nd
On Thu, Feb 09, 2017 at 03:41:41PM +0000, Mihail Atanassov wrote:
> Hi,
>
> I was working on a few patches adding fields to struct malidp_crtc_state and
> found myself writing memcpy multiple times in the ->atomic_duplicate_state
> hook because I wanted to avoid copying the drm_crtc_state twice
> (__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
> also applies to the other drm_*_state derivatives, so I concocted a macro
> helper to do the copy in one chunk (two if you count the __drm_atomic_helper*
> one). I'd appreciate some comments on whether anyone else might find this
> macro useful. Thanks!
>
> Mihail Atanassov (1):
> drm: Add helper macro for duplicating custom drm_*_state
I don't have your patch here, did something go wrong with the submission?
Only the cover letter seems to have made it through ...
-Daniel
>
> include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> --
> Mihail Atanassov
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC] drm: Helper macro for drm state duplication
@ 2017-02-09 16:57 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-02-09 16:57 UTC (permalink / raw)
To: Mihail Atanassov; +Cc: Daniel Vetter, nd, Liviu Dudau, linux-kernel, dri-devel
On Thu, Feb 09, 2017 at 03:41:41PM +0000, Mihail Atanassov wrote:
> Hi,
>
> I was working on a few patches adding fields to struct malidp_crtc_state and
> found myself writing memcpy multiple times in the ->atomic_duplicate_state
> hook because I wanted to avoid copying the drm_crtc_state twice
> (__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
> also applies to the other drm_*_state derivatives, so I concocted a macro
> helper to do the copy in one chunk (two if you count the __drm_atomic_helper*
> one). I'd appreciate some comments on whether anyone else might find this
> macro useful. Thanks!
>
> Mihail Atanassov (1):
> drm: Add helper macro for duplicating custom drm_*_state
I don't have your patch here, did something go wrong with the submission?
Only the cover letter seems to have made it through ...
-Daniel
>
> include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> --
> Mihail Atanassov
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC] drm: Helper macro for drm state duplication
2017-02-09 16:57 ` Daniel Vetter
@ 2017-02-09 17:18 ` Liviu Dudau
-1 siblings, 0 replies; 12+ messages in thread
From: Liviu Dudau @ 2017-02-09 17:18 UTC (permalink / raw)
To: Mihail Atanassov, dri-devel, linux-kernel, Daniel Vetter, nd
On Thu, Feb 09, 2017 at 05:57:04PM +0100, Daniel Vetter wrote:
> On Thu, Feb 09, 2017 at 03:41:41PM +0000, Mihail Atanassov wrote:
> > Hi,
> >
> > I was working on a few patches adding fields to struct malidp_crtc_state and
> > found myself writing memcpy multiple times in the ->atomic_duplicate_state
> > hook because I wanted to avoid copying the drm_crtc_state twice
> > (__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
> > also applies to the other drm_*_state derivatives, so I concocted a macro
> > helper to do the copy in one chunk (two if you count the __drm_atomic_helper*
> > one). I'd appreciate some comments on whether anyone else might find this
> > macro useful. Thanks!
> >
> > Mihail Atanassov (1):
> > drm: Add helper macro for duplicating custom drm_*_state
>
> I don't have your patch here, did something go wrong with the submission?
> Only the cover letter seems to have made it through ...
There is another patch that should come through, I can see it in my home account.
Liviu
> -Daniel
>
> >
> > include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > --
> > Mihail Atanassov
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC] drm: Helper macro for drm state duplication
@ 2017-02-09 17:18 ` Liviu Dudau
0 siblings, 0 replies; 12+ messages in thread
From: Liviu Dudau @ 2017-02-09 17:18 UTC (permalink / raw)
To: Mihail Atanassov, dri-devel, linux-kernel, Daniel Vetter, nd
On Thu, Feb 09, 2017 at 05:57:04PM +0100, Daniel Vetter wrote:
> On Thu, Feb 09, 2017 at 03:41:41PM +0000, Mihail Atanassov wrote:
> > Hi,
> >
> > I was working on a few patches adding fields to struct malidp_crtc_state and
> > found myself writing memcpy multiple times in the ->atomic_duplicate_state
> > hook because I wanted to avoid copying the drm_crtc_state twice
> > (__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
> > also applies to the other drm_*_state derivatives, so I concocted a macro
> > helper to do the copy in one chunk (two if you count the __drm_atomic_helper*
> > one). I'd appreciate some comments on whether anyone else might find this
> > macro useful. Thanks!
> >
> > Mihail Atanassov (1):
> > drm: Add helper macro for duplicating custom drm_*_state
>
> I don't have your patch here, did something go wrong with the submission?
> Only the cover letter seems to have made it through ...
There is another patch that should come through, I can see it in my home account.
Liviu
> -Daniel
>
> >
> > include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > --
> > Mihail Atanassov
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm: Add helper macro for duplicating custom drm_*_state
2017-02-09 15:41 ` Mihail Atanassov
@ 2017-02-09 17:35 ` Daniel Vetter
-1 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-02-09 17:35 UTC (permalink / raw)
To: Mihail Atanassov; +Cc: dri-devel, Liviu Dudau, linux-kernel, Daniel Vetter, nd
On Thu, Feb 09, 2017 at 03:41:42PM +0000, Mihail Atanassov wrote:
> Assuming a derived struct of the form:
>
> struct foo_bar_state
> {
> struct drm_bar_state bar_state;
> struct foo_private priv;
> struct foo_private2 *priv2;
> };
>
> memcpy priv and priv2 to the new instance of foo_bar_state. The
> intention is to use this macro in ->atomic_duplicate_state in conjunction with
> __drm_atomic_helper_*_duplicate_state, which already copies the relevant
> drm_*_state struct.
>
> There's an equality check for new_state and old_state to ensure that
> they are distinct instances of the same type, and a BUILD_BUG if the
> base struct (bar_state in the above example) is not first in the derived
> struct, to avoid missing any data before it and corrupting the base's data.
>
> Signed-off-by: Mihail Atanassov <mihail.atanassov@arm.com>
Seems reasonable, but I don't have any strong opinions about it's
usefulness. Converting a few drivers (to really establish it as a
pattern), or at least a few acks from maintainers, to prove that it's a
good idea, and I'll merge this.
-Daniel
> ---
> include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index d066e94..ecc6a82 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -171,6 +171,39 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> uint32_t size);
>
> /**
> + * drm_atomic_duplicate_custom_state - helper macro for duplicating
> + * driver-private additions to drm_*_state.
> + * @new_state: pointer to destination state struct
> + * @old_state: pointer to source state struct
> + * @basename: name of the drm_*_state member of the new_state/old_state struct
> + *
> + * Copies the data after the base struct until the end of the custom struct,
> + * e.g. given a structure
> + *
> + * struct foo_bar_state {
> + * struct drm_bar_state base;
> + * struct foo_private priv;
> + * struct foo_private2 *priv2;
> + * };
> + *
> + * this copies priv and priv2. NB: the base struct *must* be the first element
> + * of the derived struct, and new_state and old_state have to be two distinct
> + * instances.
> + */
> +#define drm_atomic_helper_duplicate_custom_state(new_state, old_state, basename) \
> + do { \
> + size_t base_size = sizeof(new_state->basename); \
> + size_t base_offset = offsetof(typeof(*new_state), basename); \
> + \
> + BUILD_BUG_ON(base_offset != 0); \
> + if (new_state == old_state) /* Type-check */ \
> + break; \
> + memcpy((char *)new_state + base_size, \
> + (char *)old_state + base_size, \
> + sizeof(*new_state) - base_size); \
> + } while(0)
> +
> +/**
> * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
> * @plane: the loop cursor
> * @crtc: the crtc whose planes are iterated
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm: Add helper macro for duplicating custom drm_*_state
@ 2017-02-09 17:35 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-02-09 17:35 UTC (permalink / raw)
To: Mihail Atanassov; +Cc: Daniel Vetter, nd, Liviu Dudau, linux-kernel, dri-devel
On Thu, Feb 09, 2017 at 03:41:42PM +0000, Mihail Atanassov wrote:
> Assuming a derived struct of the form:
>
> struct foo_bar_state
> {
> struct drm_bar_state bar_state;
> struct foo_private priv;
> struct foo_private2 *priv2;
> };
>
> memcpy priv and priv2 to the new instance of foo_bar_state. The
> intention is to use this macro in ->atomic_duplicate_state in conjunction with
> __drm_atomic_helper_*_duplicate_state, which already copies the relevant
> drm_*_state struct.
>
> There's an equality check for new_state and old_state to ensure that
> they are distinct instances of the same type, and a BUILD_BUG if the
> base struct (bar_state in the above example) is not first in the derived
> struct, to avoid missing any data before it and corrupting the base's data.
>
> Signed-off-by: Mihail Atanassov <mihail.atanassov@arm.com>
Seems reasonable, but I don't have any strong opinions about it's
usefulness. Converting a few drivers (to really establish it as a
pattern), or at least a few acks from maintainers, to prove that it's a
good idea, and I'll merge this.
-Daniel
> ---
> include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index d066e94..ecc6a82 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -171,6 +171,39 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> uint32_t size);
>
> /**
> + * drm_atomic_duplicate_custom_state - helper macro for duplicating
> + * driver-private additions to drm_*_state.
> + * @new_state: pointer to destination state struct
> + * @old_state: pointer to source state struct
> + * @basename: name of the drm_*_state member of the new_state/old_state struct
> + *
> + * Copies the data after the base struct until the end of the custom struct,
> + * e.g. given a structure
> + *
> + * struct foo_bar_state {
> + * struct drm_bar_state base;
> + * struct foo_private priv;
> + * struct foo_private2 *priv2;
> + * };
> + *
> + * this copies priv and priv2. NB: the base struct *must* be the first element
> + * of the derived struct, and new_state and old_state have to be two distinct
> + * instances.
> + */
> +#define drm_atomic_helper_duplicate_custom_state(new_state, old_state, basename) \
> + do { \
> + size_t base_size = sizeof(new_state->basename); \
> + size_t base_offset = offsetof(typeof(*new_state), basename); \
> + \
> + BUILD_BUG_ON(base_offset != 0); \
> + if (new_state == old_state) /* Type-check */ \
> + break; \
> + memcpy((char *)new_state + base_size, \
> + (char *)old_state + base_size, \
> + sizeof(*new_state) - base_size); \
> + } while(0)
> +
> +/**
> * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
> * @plane: the loop cursor
> * @crtc: the crtc whose planes are iterated
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm: Add helper macro for duplicating custom drm_*_state
2017-02-09 15:41 ` Mihail Atanassov
@ 2017-02-09 17:38 ` Daniel Vetter
-1 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-02-09 17:38 UTC (permalink / raw)
To: Mihail Atanassov; +Cc: dri-devel, Liviu Dudau, linux-kernel, Daniel Vetter, nd
On Thu, Feb 09, 2017 at 03:41:42PM +0000, Mihail Atanassov wrote:
> Assuming a derived struct of the form:
>
> struct foo_bar_state
> {
> struct drm_bar_state bar_state;
> struct foo_private priv;
> struct foo_private2 *priv2;
> };
>
> memcpy priv and priv2 to the new instance of foo_bar_state. The
> intention is to use this macro in ->atomic_duplicate_state in conjunction with
> __drm_atomic_helper_*_duplicate_state, which already copies the relevant
> drm_*_state struct.
>
> There's an equality check for new_state and old_state to ensure that
> they are distinct instances of the same type, and a BUILD_BUG if the
> base struct (bar_state in the above example) is not first in the derived
> struct, to avoid missing any data before it and corrupting the base's data.
>
> Signed-off-by: Mihail Atanassov <mihail.atanassov@arm.com>
> ---
> include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index d066e94..ecc6a82 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -171,6 +171,39 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> uint32_t size);
>
> /**
> + * drm_atomic_duplicate_custom_state - helper macro for duplicating
> + * driver-private additions to drm_*_state.
> + * @new_state: pointer to destination state struct
> + * @old_state: pointer to source state struct
> + * @basename: name of the drm_*_state member of the new_state/old_state struct
> + *
> + * Copies the data after the base struct until the end of the custom struct,
> + * e.g. given a structure
> + *
> + * struct foo_bar_state {
> + * struct drm_bar_state base;
> + * struct foo_private priv;
> + * struct foo_private2 *priv2;
> + * };
Forgot the kernel-doc bikeshed: Please reformat this that it's a proper
rst quoted block for prettier output. See
http://blog.ffwll.ch/2016/12/howto-docs.html
> + *
> + * this copies priv and priv2. NB: the base struct *must* be the first element
Iirc rst has a different opinion how bold higlighting works, pls double
check this is the right one.
> + * of the derived struct, and new_state and old_state have to be two distinct
@new_state and @old_state
> + * instances.
btw for even more fanciness you could do an example of what a driver's
foo_bar_duplicate_state() function would look like, using this. That would
be even better I think. And maybe s/foo/drv/ for clarity ...
Just ideas, feel free to prettify as you see fit.
Cheers, Daniel
> + */
> +#define drm_atomic_helper_duplicate_custom_state(new_state, old_state, basename) \
> + do { \
> + size_t base_size = sizeof(new_state->basename); \
> + size_t base_offset = offsetof(typeof(*new_state), basename); \
> + \
> + BUILD_BUG_ON(base_offset != 0); \
> + if (new_state == old_state) /* Type-check */ \
> + break; \
> + memcpy((char *)new_state + base_size, \
> + (char *)old_state + base_size, \
> + sizeof(*new_state) - base_size); \
> + } while(0)
> +
> +/**
> * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
> * @plane: the loop cursor
> * @crtc: the crtc whose planes are iterated
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm: Add helper macro for duplicating custom drm_*_state
@ 2017-02-09 17:38 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-02-09 17:38 UTC (permalink / raw)
To: Mihail Atanassov; +Cc: Daniel Vetter, nd, Liviu Dudau, linux-kernel, dri-devel
On Thu, Feb 09, 2017 at 03:41:42PM +0000, Mihail Atanassov wrote:
> Assuming a derived struct of the form:
>
> struct foo_bar_state
> {
> struct drm_bar_state bar_state;
> struct foo_private priv;
> struct foo_private2 *priv2;
> };
>
> memcpy priv and priv2 to the new instance of foo_bar_state. The
> intention is to use this macro in ->atomic_duplicate_state in conjunction with
> __drm_atomic_helper_*_duplicate_state, which already copies the relevant
> drm_*_state struct.
>
> There's an equality check for new_state and old_state to ensure that
> they are distinct instances of the same type, and a BUILD_BUG if the
> base struct (bar_state in the above example) is not first in the derived
> struct, to avoid missing any data before it and corrupting the base's data.
>
> Signed-off-by: Mihail Atanassov <mihail.atanassov@arm.com>
> ---
> include/drm/drm_atomic_helper.h | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index d066e94..ecc6a82 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -171,6 +171,39 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> uint32_t size);
>
> /**
> + * drm_atomic_duplicate_custom_state - helper macro for duplicating
> + * driver-private additions to drm_*_state.
> + * @new_state: pointer to destination state struct
> + * @old_state: pointer to source state struct
> + * @basename: name of the drm_*_state member of the new_state/old_state struct
> + *
> + * Copies the data after the base struct until the end of the custom struct,
> + * e.g. given a structure
> + *
> + * struct foo_bar_state {
> + * struct drm_bar_state base;
> + * struct foo_private priv;
> + * struct foo_private2 *priv2;
> + * };
Forgot the kernel-doc bikeshed: Please reformat this that it's a proper
rst quoted block for prettier output. See
http://blog.ffwll.ch/2016/12/howto-docs.html
> + *
> + * this copies priv and priv2. NB: the base struct *must* be the first element
Iirc rst has a different opinion how bold higlighting works, pls double
check this is the right one.
> + * of the derived struct, and new_state and old_state have to be two distinct
@new_state and @old_state
> + * instances.
btw for even more fanciness you could do an example of what a driver's
foo_bar_duplicate_state() function would look like, using this. That would
be even better I think. And maybe s/foo/drv/ for clarity ...
Just ideas, feel free to prettify as you see fit.
Cheers, Daniel
> + */
> +#define drm_atomic_helper_duplicate_custom_state(new_state, old_state, basename) \
> + do { \
> + size_t base_size = sizeof(new_state->basename); \
> + size_t base_offset = offsetof(typeof(*new_state), basename); \
> + \
> + BUILD_BUG_ON(base_offset != 0); \
> + if (new_state == old_state) /* Type-check */ \
> + break; \
> + memcpy((char *)new_state + base_size, \
> + (char *)old_state + base_size, \
> + sizeof(*new_state) - base_size); \
> + } while(0)
> +
> +/**
> * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
> * @plane: the loop cursor
> * @crtc: the crtc whose planes are iterated
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-02-09 18:35 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-09 15:41 [RFC] drm: Helper macro for drm state duplication Mihail Atanassov
2017-02-09 15:41 ` Mihail Atanassov
2017-02-09 15:41 ` [PATCH] drm: Add helper macro for duplicating custom drm_*_state Mihail Atanassov
2017-02-09 15:41 ` Mihail Atanassov
2017-02-09 17:35 ` Daniel Vetter
2017-02-09 17:35 ` Daniel Vetter
2017-02-09 17:38 ` Daniel Vetter
2017-02-09 17:38 ` Daniel Vetter
2017-02-09 16:57 ` [RFC] drm: Helper macro for drm state duplication Daniel Vetter
2017-02-09 16:57 ` Daniel Vetter
2017-02-09 17:18 ` Liviu Dudau
2017-02-09 17:18 ` Liviu Dudau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.