| Message ID | 20190118171106.11188-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Fri, Jan 18, 2019 at 07:11:06PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > v2: Leave the stride alone for buffers that look to be for the cursor > --- > drivers/gpu/drm/i915/i915_gem.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 1e7c95d0fea1..b4b34519af80 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -745,7 +745,12 @@ i915_gem_dumb_create(struct drm_file *file, > struct drm_mode_create_dumb *args) > { > /* have to work out size/pitch and return them */ > - args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); > + if (args->bpp == 32 && (args->width == 64 || > + args->width == 128 || > + args->width == 256)) > + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); > + else > + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 4096); Shouldn't we convert this into a non-hack and just do it anytime the pitch is too wide for the display to handle? Or am I missing something somewhere? -modesetting et all will dtrt because tiling, but this should help with boot splashes and stuff like that (but those tend to not do side-by-side, so maybe that's why the get away with it). -Daniel > args->size = args->pitch * args->height; > return i915_gem_create(file, to_i915(dev), > args->size, &args->handle); > -- > 2.19.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Jan 30, 2019 at 10:54:15AM +0100, Daniel Vetter wrote: > On Fri, Jan 18, 2019 at 07:11:06PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > v2: Leave the stride alone for buffers that look to be for the cursor > > --- > > drivers/gpu/drm/i915/i915_gem.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index 1e7c95d0fea1..b4b34519af80 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -745,7 +745,12 @@ i915_gem_dumb_create(struct drm_file *file, > > struct drm_mode_create_dumb *args) > > { > > /* have to work out size/pitch and return them */ > > - args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); > > + if (args->bpp == 32 && (args->width == 64 || > > + args->width == 128 || > > + args->width == 256)) > > + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); > > + else > > + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 4096); > > Shouldn't we convert this into a non-hack and just do it anytime the pitch > is too wide for the display to handle? > > Or am I missing something somewhere? -modesetting et all will dtrt > because tiling, but this should help with boot splashes and stuff like > that (but those tend to not do side-by-side, so maybe that's why the get > away with it). Correction: We need this, and before we start bumping the limits. Any dumb buffer you create (within the limits) we must be able to scan out. So we definitely need to have this for super-big buffers on gen7+. Even if it's fairly theoretical. I think we should even have an igt which creates a max size buffer (using dumb create) and then makes sure we can scan that out. -Daniel > -Daniel > > > args->size = args->pitch * args->height; > > return i915_gem_create(file, to_i915(dev), > > args->size, &args->handle); > > -- > > 2.19.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Wed, Jan 30, 2019 at 11:06:07AM +0100, Daniel Vetter wrote: > On Wed, Jan 30, 2019 at 10:54:15AM +0100, Daniel Vetter wrote: > > On Fri, Jan 18, 2019 at 07:11:06PM +0200, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > v2: Leave the stride alone for buffers that look to be for the cursor > > > --- > > > drivers/gpu/drm/i915/i915_gem.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > > index 1e7c95d0fea1..b4b34519af80 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem.c > > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > > @@ -745,7 +745,12 @@ i915_gem_dumb_create(struct drm_file *file, > > > struct drm_mode_create_dumb *args) > > > { > > > /* have to work out size/pitch and return them */ > > > - args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); > > > + if (args->bpp == 32 && (args->width == 64 || > > > + args->width == 128 || > > > + args->width == 256)) > > > + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); > > > + else > > > + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 4096); > > > > Shouldn't we convert this into a non-hack and just do it anytime the pitch > > is too wide for the display to handle? > > > > Or am I missing something somewhere? -modesetting et all will dtrt > > because tiling, but this should help with boot splashes and stuff like > > that (but those tend to not do side-by-side, so maybe that's why the get > > away with it). > > Correction: We need this, and before we start bumping the limits. Any dumb > buffer you create (within the limits) we must be able to scan out. So we > definitely need to have this for super-big buffers on gen7+. Even if it's > fairly theoretical. Is there some userpsace that actually wants to create a huge dumb buffer like that? Looks like we don't even check against the max fb dimensions in dumb_create so you can create any sized dumb buffer you want. So even now there is no guarantee that the addfb will actually succeed. > > I think we should even have an igt which creates a max size buffer (using > dumb create) and then makes sure we can scan that out. > -Daniel > > > -Daniel > > > > > args->size = args->pitch * args->height; > > > return i915_gem_create(file, to_i915(dev), > > > args->size, &args->handle); > > > -- > > > 2.19.2 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Wed, Jan 30, 2019 at 05:38:03PM +0200, Ville Syrjälä wrote: > On Wed, Jan 30, 2019 at 11:06:07AM +0100, Daniel Vetter wrote: > > On Wed, Jan 30, 2019 at 10:54:15AM +0100, Daniel Vetter wrote: > > > On Fri, Jan 18, 2019 at 07:11:06PM +0200, Ville Syrjala wrote: > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > v2: Leave the stride alone for buffers that look to be for the cursor > > > > --- > > > > drivers/gpu/drm/i915/i915_gem.c | 7 ++++++- > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > > > index 1e7c95d0fea1..b4b34519af80 100644 > > > > --- a/drivers/gpu/drm/i915/i915_gem.c > > > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > > > @@ -745,7 +745,12 @@ i915_gem_dumb_create(struct drm_file *file, > > > > struct drm_mode_create_dumb *args) > > > > { > > > > /* have to work out size/pitch and return them */ > > > > - args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); > > > > + if (args->bpp == 32 && (args->width == 64 || > > > > + args->width == 128 || > > > > + args->width == 256)) > > > > + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); > > > > + else > > > > + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 4096); > > > > > > Shouldn't we convert this into a non-hack and just do it anytime the pitch > > > is too wide for the display to handle? > > > > > > Or am I missing something somewhere? -modesetting et all will dtrt > > > because tiling, but this should help with boot splashes and stuff like > > > that (but those tend to not do side-by-side, so maybe that's why the get > > > away with it). > > > > Correction: We need this, and before we start bumping the limits. Any dumb > > buffer you create (within the limits) we must be able to scan out. So we > > definitely need to have this for super-big buffers on gen7+. Even if it's > > fairly theoretical. > > Is there some userpsace that actually wants to create a huge dumb > buffer like that? Given that the core drm_mode_dumb_create doesn't check anything, I guess not. Would be good to add. > Looks like we don't even check against the max fb dimensions in > dumb_create so you can create any sized dumb buffer you want. > So even now there is no guarantee that the addfb will actually > succeed. Well we do tell userspace up to what limit we expect it to succeed, and dumb_create is supposed to come up with the correct stride. I think there's no good reason to give userspace a stride that doesn't work at all, and it's essentially this patch, but with a slightly different if condition. -Daniel > > > > > I think we should even have an igt which creates a max size buffer (using > > dumb create) and then makes sure we can scan that out. > > -Daniel > > > > > -Daniel > > > > > > > args->size = args->pitch * args->height; > > > > return i915_gem_create(file, to_i915(dev), > > > > args->size, &args->handle); > > > > -- > > > > 2.19.2 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 1e7c95d0fea1..b4b34519af80 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -745,7 +745,12 @@ i915_gem_dumb_create(struct drm_file *file, struct drm_mode_create_dumb *args) { /* have to work out size/pitch and return them */ - args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); + if (args->bpp == 32 && (args->width == 64 || + args->width == 128 || + args->width == 256)) + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64); + else + args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 4096); args->size = args->pitch * args->height; return i915_gem_create(file, to_i915(dev), args->size, &args->handle);
From: Ville Syrjälä <ville.syrjala@linux.intel.com> v2: Leave the stride alone for buffers that look to be for the cursor --- drivers/gpu/drm/i915/i915_gem.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)