On Thu, Aug 16, 2018 at 12:42:59PM +0200, Daniel Vetter wrote: > On Thu, Aug 16, 2018 at 12:25 PM, Liviu Dudau <liviu.dudau@xxxxxxx> wrote: > > On Wed, Aug 15, 2018 at 03:40:17PM +0200, Daniel Vetter wrote: > >> 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. > > > > Another option would be to add to the drm_framebuffer_funcs structure a > > hook that drivers could implement that does the validation of the fb and I meant to say "does the validation of the fb creation". > > could be used to bypass the more strict check in drm_gem_fb_create_fb_with_funcs() > > Can't call an fb operation if you don't yet have an fb :-) The funcs get passed as an argument to the _with_funcs() drm_gem_fb_create variant, so the hook should be available. And it is a validation operation, I would say it should be allowed even if the fb is not yet present. Best regards, Liviu > -Daniel > > > > > Best regards, > > Liviu > > > >> -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 > > > > -- > > ==================== > > | I would like to | > > | fix the world, | > > | but they're not | > > | giving me the | > > \ source code! / > > --------------- > > ¯\_(ツ)_/¯ > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
