| Message ID | 20200714201945.18959-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/fbc: Limit cfb to the first 256MiB of stolen on g4x+ | expand |
Quoting Ville Syrjala (2020-07-14 21:19:45) > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Since g4x the CFB base only takes a 28bit offset into stolen. > Not sure if the CFB is allowed to start below that limit but > then extend beyond it. Let's assume not and just restrict the > allocation to the first 256MiB (in the unlikely case > we have more stolen than that). > > v2: s/BIT/BIT_ULL/ (Chris) > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_fbc.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > index 85723fba6002..3a4f980788a6 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -424,6 +424,14 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv, > fbc->no_fbc_reason = reason; > } > > +static u64 intel_fbc_cfb_base_max(struct drm_i915_private *i915) > +{ > + if (INTEL_GEN(i915) >= 5 || IS_G4X(i915)) > + return BIT_ULL(28); > + else > + return BIT_ULL(32); > +} Confirmed that ilk uses 23:12. I trust g4x is the same. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> I didn't find the others quickly, but it's not going to harm. -Chris
On Tue, Jul 14, 2020 at 09:32:54PM +0100, Chris Wilson wrote: > Quoting Ville Syrjala (2020-07-14 21:19:45) > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Since g4x the CFB base only takes a 28bit offset into stolen. > > Not sure if the CFB is allowed to start below that limit but > > then extend beyond it. Let's assume not and just restrict the > > allocation to the first 256MiB (in the unlikely case > > we have more stolen than that). > > > > v2: s/BIT/BIT_ULL/ (Chris) > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_fbc.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > > index 85723fba6002..3a4f980788a6 100644 > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > @@ -424,6 +424,14 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv, > > fbc->no_fbc_reason = reason; > > } > > > > +static u64 intel_fbc_cfb_base_max(struct drm_i915_private *i915) > > +{ > > + if (INTEL_GEN(i915) >= 5 || IS_G4X(i915)) > > + return BIT_ULL(28); > > + else > > + return BIT_ULL(32); > > +} > > Confirmed that ilk uses 23:12. I trust g4x is the same. I guess you mean 27:12? Or did you find a some docs saying it's only 24 bits? All the docs I have say 27:12. And just for the heck of it I tested on real hw by writing ~0 to the register. This gave me the following results: snb/ivb/kbl: 0x0ffff000 -> agrees with the docs g4x/ilk: 0xfffff0f0 -> kinda looks like it's trying to be a 36bit address cl: 0xffffffff -> can't make any real conclusions So the g4x/ilk case is a bit strange. Maybe they initially planned to make it take 36bit physical addresses and then changed their minds and just made it an offset into stolen. Would need actual testing to confirm whether >28bit offsets work. Too lazy to do that atm so limiting to 28bits as per the docs is the safe route. Also >32bits would seem rather pointless anyway as I don't think stolen can live above 4GiB on these platforms. > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > I didn't find the others quickly, but it's not going to harm. > -Chris
Quoting Ville Syrjälä (2020-07-15 15:22:24) > On Tue, Jul 14, 2020 at 09:32:54PM +0100, Chris Wilson wrote: > > Quoting Ville Syrjala (2020-07-14 21:19:45) > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Since g4x the CFB base only takes a 28bit offset into stolen. > > > Not sure if the CFB is allowed to start below that limit but > > > then extend beyond it. Let's assume not and just restrict the > > > allocation to the first 256MiB (in the unlikely case > > > we have more stolen than that). > > > > > > v2: s/BIT/BIT_ULL/ (Chris) > > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_fbc.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c > > > index 85723fba6002..3a4f980788a6 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > > @@ -424,6 +424,14 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv, > > > fbc->no_fbc_reason = reason; > > > } > > > > > > +static u64 intel_fbc_cfb_base_max(struct drm_i915_private *i915) > > > +{ > > > + if (INTEL_GEN(i915) >= 5 || IS_G4X(i915)) > > > + return BIT_ULL(28); > > > + else > > > + return BIT_ULL(32); > > > +} > > > > Confirmed that ilk uses 23:12. I trust g4x is the same. > > I guess you mean 27:12? Or did you find a some docs saying it's only > 24 bits? All the docs I have say 27:12. Typo, 27:12 from DPFC_CB_BASE. -Chris
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 85723fba6002..3a4f980788a6 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -424,6 +424,14 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv, fbc->no_fbc_reason = reason; } +static u64 intel_fbc_cfb_base_max(struct drm_i915_private *i915) +{ + if (INTEL_GEN(i915) >= 5 || IS_G4X(i915)) + return BIT_ULL(28); + else + return BIT_ULL(32); +} + static int find_compression_threshold(struct drm_i915_private *dev_priv, struct drm_mm_node *node, unsigned int size, @@ -442,6 +450,8 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv, else end = U64_MAX; + end = min(end, intel_fbc_cfb_base_max(dev_priv)); + /* HACK: This code depends on what we will do in *_enable_fbc. If that * code changes, this code needs to change as well. *