On Fri, 22 May 2020 at 18:48, Sam Ravnborg <sam@xxxxxxxxxxxx> wrote: > > Hi Thomas. > > On Fri, May 22, 2020 at 03:52:26PM +0200, Thomas Zimmermann wrote: > > Rename the macro to DRM_GEM_CMA_DRIVER_OPS to align with SHMEM > > helpers. > This part is fine, I like that the naming is somehow consistent. > > > An internal version is provided for drivers that override > > the default .dumb_create callback. Adapt drivers to the changes. > I loathe anything named __foo or __FOO. This __ signals to me > that the author was clueless in naming - or some sort. > I know that __ is used in some lib headers - but thats not the case > here. > > But I love that we have a variant that takes a create function. > So we do not have to escape from the nice macro. > The macro is another way to tell me as rewiewer that this > drivers uses all the default helpers for this. > Fwiw I share the sentiment, although I fear we're a little late. __ prefixed functions are widely common in core drm and it's helpers. > > So critizising the name I better suggest something that > I personally like better: > > DRM_GEM_CMA_DRIVER_OPS_CREATE() > > It would look like this: > /* GEM Operations */ > - DRM_GEM_CMA_VMAP_DRIVER_OPS, > - .dumb_create = drm_sun4i_gem_dumb_create, > + DRM_GEM_CMA_DRIVER_OPS_CREATE(drm_sun4i_gem_dumb_create), > > > > Please fix zte/zx_drm_drv.c which also uses DRM_GEM_CMA_VMAP_DRIVER_OPS. > Isn't DRM_GEM_CMA_VMAP_DRIVER_OPS introduced to zte with the last patch in the series? -Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel