Re: [Intel-gfx] [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k
On Thu, Sep 20, 2018 at 08:46:42PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-09-20 20:41:30)
> > On Thu, Sep 20, 2018 at 09:09:03AM +0100, Chris Wilson wrote:
> > > Quoting Ville Syrjälä (2018-09-19 17:59:51)
> > > > On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä
> > > > >
> > > > > With gtt remapping in place we can use arbitraily large framebuffers.
> > > > > Let's bump the limits as high as we can (32k-1). Going beyond that
> > > > > would require switching our s16.16 src coordinate representation to
> > > > > something with more spare bits.
> > > > >
> > > > > Signed-off-by: Ville Syrjälä
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > index 346572cf734a..0ee6255cd040 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
> > > > > dev->mode_config.max_width = 4096;
> > > > > dev->mode_config.max_height = 4096;
> > > > > } else {
> > > > > - dev->mode_config.max_width = 8192;
> > > > > - dev->mode_config.max_height = 8192;
> > > > > + dev->mode_config.max_width = 32767;
> > > > > + dev->mode_config.max_height = 32767;
> > > >
> > > > It appears that neither Mesa nor glamor will check whether window system
> > > > buffers exceed the capabilities of the 3D engine. So trying to use a
> > > > >16k
> > > > trips an assert when genxml tries to pack the surface_state.
> > > >
> > > > So looks like we'll need to limit this to 16k for gen7+, and leave it
> > > > at 8k for gen4+. If userspace gets smarter later on we could add a new
> > > > client cap to expose higher limits.
> > >
> > > At which point, the client can just ignore this field and just use
> > > rejection criteria from addfb2 and/or setcrtc (or the atomic variant).
> >
> > I suppose. Though probing the max size using addfb might be a bit
> > tedious. That's assuming the client wants to report the max in some
> > way, as X does.
> >
> > >
> > > Or we can just keep this field as meaning the maximum size of a single
> > > CRTC and just ignore it entirely in -modesetting for fb size as we do
> > > elsewhere.
> >
> > Would require changing the core addfb code to ignore these
> > limits for i915 but keep chekcing them for the other drivers.
> > So a bit of work, and I'm not really sure what the actual
> > benefit for i915 would be.
>
> Why is the core addfb using these fields? Since when did they *stop*
> being per-CRTC limits?
Looks like addfb has been using them since the beginning:
commit f453ba0460742ad027ae0c4c7d61e62817b3e7ef
Author: Dave Airlie
Date: Fri Nov 7 14:05:41 2008 -0800
DRM: add mode setting support
And looks like they never matched the per-crtc limits for i915 either.
Until HSW hdisplay limit was 4k but max_width was already set to 8k
in:
commit 79e539453b34e35f39299a899d263b0a1f1670bd
Author: Jesse Barnes
Date: Fri Nov 7 14:24:08 2008 -0800
DRM: i915: add mode setting support
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k
Quoting Ville Syrjälä (2018-09-20 20:41:30)
> On Thu, Sep 20, 2018 at 09:09:03AM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjälä (2018-09-19 17:59:51)
> > > On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä
> > > >
> > > > With gtt remapping in place we can use arbitraily large framebuffers.
> > > > Let's bump the limits as high as we can (32k-1). Going beyond that
> > > > would require switching our s16.16 src coordinate representation to
> > > > something with more spare bits.
> > > >
> > > > Signed-off-by: Ville Syrjälä
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index 346572cf734a..0ee6255cd040 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
> > > > dev->mode_config.max_width = 4096;
> > > > dev->mode_config.max_height = 4096;
> > > > } else {
> > > > - dev->mode_config.max_width = 8192;
> > > > - dev->mode_config.max_height = 8192;
> > > > + dev->mode_config.max_width = 32767;
> > > > + dev->mode_config.max_height = 32767;
> > >
> > > It appears that neither Mesa nor glamor will check whether window system
> > > buffers exceed the capabilities of the 3D engine. So trying to use a >16k
> > > trips an assert when genxml tries to pack the surface_state.
> > >
> > > So looks like we'll need to limit this to 16k for gen7+, and leave it
> > > at 8k for gen4+. If userspace gets smarter later on we could add a new
> > > client cap to expose higher limits.
> >
> > At which point, the client can just ignore this field and just use
> > rejection criteria from addfb2 and/or setcrtc (or the atomic variant).
>
> I suppose. Though probing the max size using addfb might be a bit
> tedious. That's assuming the client wants to report the max in some
> way, as X does.
>
> >
> > Or we can just keep this field as meaning the maximum size of a single
> > CRTC and just ignore it entirely in -modesetting for fb size as we do
> > elsewhere.
>
> Would require changing the core addfb code to ignore these
> limits for i915 but keep chekcing them for the other drivers.
> So a bit of work, and I'm not really sure what the actual
> benefit for i915 would be.
Why is the core addfb using these fields? Since when did they *stop*
being per-CRTC limits?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k
On Thu, Sep 20, 2018 at 09:09:03AM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-09-19 17:59:51)
> > On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä
> > >
> > > With gtt remapping in place we can use arbitraily large framebuffers.
> > > Let's bump the limits as high as we can (32k-1). Going beyond that
> > > would require switching our s16.16 src coordinate representation to
> > > something with more spare bits.
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 346572cf734a..0ee6255cd040 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
> > > dev->mode_config.max_width = 4096;
> > > dev->mode_config.max_height = 4096;
> > > } else {
> > > - dev->mode_config.max_width = 8192;
> > > - dev->mode_config.max_height = 8192;
> > > + dev->mode_config.max_width = 32767;
> > > + dev->mode_config.max_height = 32767;
> >
> > It appears that neither Mesa nor glamor will check whether window system
> > buffers exceed the capabilities of the 3D engine. So trying to use a >16k
> > trips an assert when genxml tries to pack the surface_state.
> >
> > So looks like we'll need to limit this to 16k for gen7+, and leave it
> > at 8k for gen4+. If userspace gets smarter later on we could add a new
> > client cap to expose higher limits.
>
> At which point, the client can just ignore this field and just use
> rejection criteria from addfb2 and/or setcrtc (or the atomic variant).
I suppose. Though probing the max size using addfb might be a bit
tedious. That's assuming the client wants to report the max in some
way, as X does.
>
> Or we can just keep this field as meaning the maximum size of a single
> CRTC and just ignore it entirely in -modesetting for fb size as we do
> elsewhere.
Would require changing the core addfb code to ignore these
limits for i915 but keep chekcing them for the other drivers.
So a bit of work, and I'm not really sure what the actual
benefit for i915 would be.
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k
Quoting Ville Syrjälä (2018-09-19 17:59:51)
> On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > With gtt remapping in place we can use arbitraily large framebuffers.
> > Let's bump the limits as high as we can (32k-1). Going beyond that
> > would require switching our s16.16 src coordinate representation to
> > something with more spare bits.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 346572cf734a..0ee6255cd040 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
> > dev->mode_config.max_width = 4096;
> > dev->mode_config.max_height = 4096;
> > } else {
> > - dev->mode_config.max_width = 8192;
> > - dev->mode_config.max_height = 8192;
> > + dev->mode_config.max_width = 32767;
> > + dev->mode_config.max_height = 32767;
>
> It appears that neither Mesa nor glamor will check whether window system
> buffers exceed the capabilities of the 3D engine. So trying to use a >16k
> trips an assert when genxml tries to pack the surface_state.
>
> So looks like we'll need to limit this to 16k for gen7+, and leave it
> at 8k for gen4+. If userspace gets smarter later on we could add a new
> client cap to expose higher limits.
At which point, the client can just ignore this field and just use
rejection criteria from addfb2 and/or setcrtc (or the atomic variant).
Or we can just keep this field as meaning the maximum size of a single
CRTC and just ignore it entirely in -modesetting for fb size as we do
elsewhere.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k
On Thu, Sep 13, 2018 at 11:01:40PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> With gtt remapping in place we can use arbitraily large framebuffers.
> Let's bump the limits as high as we can (32k-1). Going beyond that
> would require switching our s16.16 src coordinate representation to
> something with more spare bits.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 346572cf734a..0ee6255cd040 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
> dev->mode_config.max_width = 4096;
> dev->mode_config.max_height = 4096;
> } else {
> - dev->mode_config.max_width = 8192;
> - dev->mode_config.max_height = 8192;
> + dev->mode_config.max_width = 32767;
> + dev->mode_config.max_height = 32767;
It appears that neither Mesa nor glamor will check whether window system
buffers exceed the capabilities of the 3D engine. So trying to use a >16k
trips an assert when genxml tries to pack the surface_state.
So looks like we'll need to limit this to 16k for gen7+, and leave it
at 8k for gen4+. If userspace gets smarter later on we could add a new
client cap to expose higher limits.
If I'm reading the spec correctly gen4+ render engine has a stride
limit of 512KiB and gen7+ has 256KiB. So my choice of 256KiB seems
good enough for both.
> }
>
> if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
> --
> 2.16.4
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 5/5] drm/i915: Bump gen4+ fb size limits to 32kx32k
From: Ville Syrjälä
With gtt remapping in place we can use arbitraily large framebuffers.
Let's bump the limits as high as we can (32k-1). Going beyond that
would require switching our s16.16 src coordinate representation to
something with more spare bits.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 346572cf734a..0ee6255cd040 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15527,8 +15527,8 @@ int intel_modeset_init(struct drm_device *dev)
dev->mode_config.max_width = 4096;
dev->mode_config.max_height = 4096;
} else {
- dev->mode_config.max_width = 8192;
- dev->mode_config.max_height = 8192;
+ dev->mode_config.max_width = 32767;
+ dev->mode_config.max_height = 32767;
}
if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
--
2.16.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
