On Wed, Aug 15, 2018 at 2:27 PM, Alexandru-Cosmin Gheorghe <Alexandru-Cosmin.Gheorghe@xxxxxxx> wrote: > On Wed, Aug 15, 2018 at 01:54:12PM +0200, Daniel Vetter wrote: >> On Wed, Aug 15, 2018 at 12:08:38PM +0100, Liviu Dudau wrote: >> > On Thu, Jul 26, 2018 at 03:10:04PM +0100, Alexandru Gheorghe wrote: >> > > Some drivers can't use drm_gem_fb_create, so instead of copying the >> > > logic that does the framebuffer allocation allow them to use core >> > > drm_gem_fb_alloc. >> > > >> > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx> >> > >> > To me it looks like an useful thing to have exported, so for what is worth: >> > >> > Acked-by: Liviu Dudau <liviu.dudau@xxxxxxx> >> > >> > Sean, Maarten, Laurent, Gustavo, Daniel(s), do you have any objections? >> >> In general please add meaningful kernel-doc for exported functions, >> explaing what's different and how it works together. >> >> Wrt the specific issue, I think we should teach drm core and helpers how >> to allocate formats with tiled blocks. Means we need to extend >> drm_format_info a bit. Your YUV formats won't be the only ones where the >> format itself arranges pixels in blocks, and hence has format-based >> alignment criteria for pitch and size (due to line rounding). > > Something like a tile_size or do you have other ideas ? > Any idea if there are any tile formats out there where the tile it's > not a square ? I think both tile_w/_h in pixels and tile_size in bytes is what you want for full description of all options. And yes I think some of the compressed GL formats aren't square. Not that I expect we'll ever have to scan those out, but who knows. This would also allow drivers to let the core check basic tiled layouts by supplying that information for a (fourcc, modifier) pair. The tricky part is then how you expect stride to be set, since that's in bytes per line, which might not align. But since all the tiles I've seen are power-of-two in height, shouldn't be an issue in practice. A check + WARN_ON would be good though. -Daniel >> I think this would also fit better into the overall design where drivers >> can overwrite the drm_format_info for specific (fourcc, modifier) >> combinations. >> -Daniel >> >> >> > >> > Best regards, >> > Liviu >> > >> > > --- >> > > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 3 ++- >> > > include/drm/drm_gem_framebuffer_helper.h | 5 +++++ >> > > 2 files changed, 7 insertions(+), 1 deletion(-) >> > > >> > > diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c >> > > index 2810d4131411..64eddf5a1bd9 100644 >> > > --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c >> > > +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c >> > > @@ -57,7 +57,7 @@ struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb, >> > > } >> > > EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj); >> > > >> > > -static struct drm_framebuffer * >> > > +struct drm_framebuffer * >> > > drm_gem_fb_alloc(struct drm_device *dev, >> > > const struct drm_mode_fb_cmd2 *mode_cmd, >> > > struct drm_gem_object **obj, unsigned int num_planes, >> > > @@ -85,6 +85,7 @@ drm_gem_fb_alloc(struct drm_device *dev, >> > > >> > > return fb; >> > > } >> > > +EXPORT_SYMBOL_GPL(drm_gem_fb_alloc); >> > > >> > > /** >> > > * drm_gem_fb_destroy - Free GEM backed framebuffer >> > > diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h >> > > index a38de7eb55b4..d20c1356000a 100644 >> > > --- a/include/drm/drm_gem_framebuffer_helper.h >> > > +++ b/include/drm/drm_gem_framebuffer_helper.h >> > > @@ -14,6 +14,11 @@ struct drm_simple_display_pipe; >> > > >> > > struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb, >> > > unsigned int plane); >> > > +struct drm_framebuffer * >> > > +drm_gem_fb_alloc(struct drm_device *dev, >> > > + const struct drm_mode_fb_cmd2 *mode_cmd, >> > > + struct drm_gem_object **obj, unsigned int num_planes, >> > > + const struct drm_framebuffer_funcs *funcs); >> > > void drm_gem_fb_destroy(struct drm_framebuffer *fb); >> > > int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file, >> > > unsigned int *handle); >> > > -- >> > > 2.18.0 >> > > >> > >> > -- >> > ==================== >> > | I would like to | >> > | fix the world, | >> > | but they're not | >> > | giving me the | >> > \ source code! / >> > --------------- >> > ¯\_(ツ)_/¯ >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> http://blog.ffwll.ch > > -- > Cheers, > Alex G -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
