| Message ID | 20191218161105.30638-8-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/tgl: Render/media decompression support | expand |
On Wed, 2019-12-18 at 18:10 +0200, Imre Deak wrote: > The CCS plane stride must be fixed on TGL, as it's not configurable > for > the display. Instead the HW has a hardwired logic to determine it > from > the main plane stride. Make sure userspace passes in the correct > stride. > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Mika Kahola <mika.kahola@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 641ea24539eb..7c52591172e1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2620,6 +2620,11 @@ bool is_ccs_modifier(u64 modifier) > modifier == I915_FORMAT_MOD_Yf_TILED_CCS; > } > > +static int gen12_ccs_aux_stride(struct drm_framebuffer *fb, int > ccs_plane) > +{ > + return DIV_ROUND_UP(fb->pitches[ccs_to_main_plane(fb, > ccs_plane)], 512) * 64; > +} > + > u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv, > u32 pixel_format, u64 modifier) > { > @@ -16530,6 +16535,16 @@ static int intel_framebuffer_init(struct > intel_framebuffer *intel_fb, > goto err; > } > > + if (is_gen12_ccs_plane(fb, i)) { > + int ccs_aux_stride = gen12_ccs_aux_stride(fb, > i); > + > + if (fb->pitches[i] != ccs_aux_stride) { > + DRM_DEBUG_KMS("ccs aux plane %d pitch > (%d) must be %d\n", > + i, fb->pitches[i], > ccs_aux_stride); > + goto err; > + } > + } > + > fb->obj[i] = &obj->base; > } >
On Wed, Dec 18, 2019 at 06:10:57PM +0200, Imre Deak wrote: > The CCS plane stride must be fixed on TGL, as it's not configurable for > the display. Instead the HW has a hardwired logic to determine it from > the main plane stride. Make sure userspace passes in the correct stride. Do you have a bspec page number reference for this? I don't see it mentioned anywhere obvious. Matt > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 641ea24539eb..7c52591172e1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2620,6 +2620,11 @@ bool is_ccs_modifier(u64 modifier) > modifier == I915_FORMAT_MOD_Yf_TILED_CCS; > } > > +static int gen12_ccs_aux_stride(struct drm_framebuffer *fb, int ccs_plane) > +{ > + return DIV_ROUND_UP(fb->pitches[ccs_to_main_plane(fb, ccs_plane)], 512) * 64; > +} > + > u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv, > u32 pixel_format, u64 modifier) > { > @@ -16530,6 +16535,16 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb, > goto err; > } > > + if (is_gen12_ccs_plane(fb, i)) { > + int ccs_aux_stride = gen12_ccs_aux_stride(fb, i); > + > + if (fb->pitches[i] != ccs_aux_stride) { > + DRM_DEBUG_KMS("ccs aux plane %d pitch (%d) must be %d\n", > + i, fb->pitches[i], ccs_aux_stride); > + goto err; > + } > + } > + > fb->obj[i] = &obj->base; > } > > -- > 2.22.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Dec 19, 2019 at 02:48:50PM -0800, Matt Roper wrote: > On Wed, Dec 18, 2019 at 06:10:57PM +0200, Imre Deak wrote: > > The CCS plane stride must be fixed on TGL, as it's not configurable for > > the display. Instead the HW has a hardwired logic to determine it from > > the main plane stride. Make sure userspace passes in the correct stride. > > Do you have a bspec page number reference for this? I don't see it > mentioned anywhere obvious. At Index/49253: "Control surface stride is not used for media compression." Assuming this refers to the stride we program to PLANE_AUX_DIST on other platforms. The description of that field tells us: "This field is unused. Leave at default value." Practice does match this, since there seems to be no significance what is programmed to this field (things keep working fine if the assumed fixed stride was used for the CCS surface whatever I programmed to the above field). So the only explanation I see is what I wrote in the commit log. The same must be true for render compression too and my test results match with that assumption. There is no mention about constraints on the stride or it being configureable somehow (unlike on other engines which use the AUX pagetables), so I also added now a ticket to the render compression page. > > > Matt > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 641ea24539eb..7c52591172e1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -2620,6 +2620,11 @@ bool is_ccs_modifier(u64 modifier) > > modifier == I915_FORMAT_MOD_Yf_TILED_CCS; > > } > > > > +static int gen12_ccs_aux_stride(struct drm_framebuffer *fb, int ccs_plane) > > +{ > > + return DIV_ROUND_UP(fb->pitches[ccs_to_main_plane(fb, ccs_plane)], 512) * 64; > > +} > > + > > u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv, > > u32 pixel_format, u64 modifier) > > { > > @@ -16530,6 +16535,16 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb, > > goto err; > > } > > > > + if (is_gen12_ccs_plane(fb, i)) { > > + int ccs_aux_stride = gen12_ccs_aux_stride(fb, i); > > + > > + if (fb->pitches[i] != ccs_aux_stride) { > > + DRM_DEBUG_KMS("ccs aux plane %d pitch (%d) must be %d\n", > > + i, fb->pitches[i], ccs_aux_stride); > > + goto err; > > + } > > + } > > + > > fb->obj[i] = &obj->base; > > } > > > > -- > > 2.22.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Matt Roper > Graphics Software Engineer > VTT-OSGC Platform Enablement > Intel Corporation > (916) 356-2795
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 641ea24539eb..7c52591172e1 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2620,6 +2620,11 @@ bool is_ccs_modifier(u64 modifier) modifier == I915_FORMAT_MOD_Yf_TILED_CCS; } +static int gen12_ccs_aux_stride(struct drm_framebuffer *fb, int ccs_plane) +{ + return DIV_ROUND_UP(fb->pitches[ccs_to_main_plane(fb, ccs_plane)], 512) * 64; +} + u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv, u32 pixel_format, u64 modifier) { @@ -16530,6 +16535,16 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb, goto err; } + if (is_gen12_ccs_plane(fb, i)) { + int ccs_aux_stride = gen12_ccs_aux_stride(fb, i); + + if (fb->pitches[i] != ccs_aux_stride) { + DRM_DEBUG_KMS("ccs aux plane %d pitch (%d) must be %d\n", + i, fb->pitches[i], ccs_aux_stride); + goto err; + } + } + fb->obj[i] = &obj->base; }
The CCS plane stride must be fixed on TGL, as it's not configurable for the display. Instead the HW has a hardwired logic to determine it from the main plane stride. Make sure userspace passes in the correct stride. Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)