| Message ID | 1486654902-2558-2-git-send-email-mihail.atanassov@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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(+)