| Message ID | 20221004103311.194409-4-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/5] drm/i915: remove the TODO in pin_and_fence_fb_obj | expand |
On Tue, Oct 04, 2022 at 11:33:10AM +0100, Matthew Auld wrote: > For these types of display buffers, we need to able to CPU access some > part of the backing memory in prepare_plane_clear_colors(). As a result > we need to ensure we always place in the mappable part of lmem, which > becomes necessary on small-bar systems. > > v2(Nirmoy & Ville): > - Add some commentary for why we need to CPU access the buffer. > - Split out the other changes, so we just consider the display change > here. > v3: > - Handle this in the dpt path. > > Fixes: eb1c535f0d69 ("drm/i915: turn on small BAR support") > Reported-by: Jianshui Yu <jianshui.yu@intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Nirmoy Das <nirmoy.das@intel.com> > --- > drivers/gpu/drm/i915/display/intel_fb_pin.c | 24 +++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c > index 32206bd359da..8197343300ee 100644 > --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c > +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c > @@ -50,7 +50,18 @@ intel_pin_fb_obj_dpt(struct drm_framebuffer *fb, > continue; > > if (HAS_LMEM(dev_priv)) { > - ret = i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0); > + unsigned int flags = obj->flags; > + > + /* > + * For this type of buffer we need to able to read from the CPU > + * the clear color value found in the buffer, hence we need to > + * ensure it is always in the mappable part of lmem, if this is > + * a small-bar device. > + */ > + if (intel_fb_rc_ccs_cc_plane(fb) >= 0) > + flags &= ~I915_BO_ALLOC_GPU_ONLY; > + ret = __i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0, > + flags); > if (ret) > continue; > } > @@ -156,8 +167,17 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, > ret = i915_gem_object_lock(obj, &ww); > if (!ret && phys_cursor) > ret = i915_gem_object_attach_phys(obj, alignment); > - else if (!ret && HAS_LMEM(dev_priv)) > + else if (!ret && HAS_LMEM(dev_priv)) { > + /* > + * For this type of ccs buffer we need to able to read from the > + * CPU the clear color value found in the buffer, which might > + * require moving to the mappable part of lmem first, but here > + * we should be using dpt for this. > + */ > + WARN_ON_ONCE(intel_fb_rc_ccs_cc_plane(fb) >= 0); DPT isn't availalable on DG1. > + > ret = i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0); > + } > if (!ret) > ret = i915_gem_object_pin_pages(obj); > if (ret) > -- > 2.37.3
On 04/10/2022 12:28, Ville Syrjälä wrote: > On Tue, Oct 04, 2022 at 11:33:10AM +0100, Matthew Auld wrote: >> For these types of display buffers, we need to able to CPU access some >> part of the backing memory in prepare_plane_clear_colors(). As a result >> we need to ensure we always place in the mappable part of lmem, which >> becomes necessary on small-bar systems. >> >> v2(Nirmoy & Ville): >> - Add some commentary for why we need to CPU access the buffer. >> - Split out the other changes, so we just consider the display change >> here. >> v3: >> - Handle this in the dpt path. >> >> Fixes: eb1c535f0d69 ("drm/i915: turn on small BAR support") >> Reported-by: Jianshui Yu <jianshui.yu@intel.com> >> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Nirmoy Das <nirmoy.das@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_fb_pin.c | 24 +++++++++++++++++++-- >> 1 file changed, 22 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c >> index 32206bd359da..8197343300ee 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c >> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c >> @@ -50,7 +50,18 @@ intel_pin_fb_obj_dpt(struct drm_framebuffer *fb, >> continue; >> >> if (HAS_LMEM(dev_priv)) { >> - ret = i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0); >> + unsigned int flags = obj->flags; >> + >> + /* >> + * For this type of buffer we need to able to read from the CPU >> + * the clear color value found in the buffer, hence we need to >> + * ensure it is always in the mappable part of lmem, if this is >> + * a small-bar device. >> + */ >> + if (intel_fb_rc_ccs_cc_plane(fb) >= 0) >> + flags &= ~I915_BO_ALLOC_GPU_ONLY; >> + ret = __i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0, >> + flags); >> if (ret) >> continue; >> } >> @@ -156,8 +167,17 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, >> ret = i915_gem_object_lock(obj, &ww); >> if (!ret && phys_cursor) >> ret = i915_gem_object_attach_phys(obj, alignment); >> - else if (!ret && HAS_LMEM(dev_priv)) >> + else if (!ret && HAS_LMEM(dev_priv)) { >> + /* >> + * For this type of ccs buffer we need to able to read from the >> + * CPU the clear color value found in the buffer, which might >> + * require moving to the mappable part of lmem first, but here >> + * we should be using dpt for this. >> + */ >> + WARN_ON_ONCE(intel_fb_rc_ccs_cc_plane(fb) >= 0); > > DPT isn't availalable on DG1. Hmm, does it also support the DG2_RC_CCS_CC modifier? > >> + >> ret = i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0); >> + } >> if (!ret) >> ret = i915_gem_object_pin_pages(obj); >> if (ret) >> -- >> 2.37.3 >
On Tue, Oct 04, 2022 at 12:51:11PM +0100, Matthew Auld wrote: > On 04/10/2022 12:28, Ville Syrjälä wrote: > > On Tue, Oct 04, 2022 at 11:33:10AM +0100, Matthew Auld wrote: > >> For these types of display buffers, we need to able to CPU access some > >> part of the backing memory in prepare_plane_clear_colors(). As a result > >> we need to ensure we always place in the mappable part of lmem, which > >> becomes necessary on small-bar systems. > >> > >> v2(Nirmoy & Ville): > >> - Add some commentary for why we need to CPU access the buffer. > >> - Split out the other changes, so we just consider the display change > >> here. > >> v3: > >> - Handle this in the dpt path. > >> > >> Fixes: eb1c535f0d69 ("drm/i915: turn on small BAR support") > >> Reported-by: Jianshui Yu <jianshui.yu@intel.com> > >> Signed-off-by: Matthew Auld <matthew.auld@intel.com> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Cc: Nirmoy Das <nirmoy.das@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_fb_pin.c | 24 +++++++++++++++++++-- > >> 1 file changed, 22 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c > >> index 32206bd359da..8197343300ee 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c > >> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c > >> @@ -50,7 +50,18 @@ intel_pin_fb_obj_dpt(struct drm_framebuffer *fb, > >> continue; > >> > >> if (HAS_LMEM(dev_priv)) { > >> - ret = i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0); > >> + unsigned int flags = obj->flags; > >> + > >> + /* > >> + * For this type of buffer we need to able to read from the CPU > >> + * the clear color value found in the buffer, hence we need to > >> + * ensure it is always in the mappable part of lmem, if this is > >> + * a small-bar device. > >> + */ > >> + if (intel_fb_rc_ccs_cc_plane(fb) >= 0) > >> + flags &= ~I915_BO_ALLOC_GPU_ONLY; > >> + ret = __i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0, > >> + flags); > >> if (ret) > >> continue; > >> } > >> @@ -156,8 +167,17 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, > >> ret = i915_gem_object_lock(obj, &ww); > >> if (!ret && phys_cursor) > >> ret = i915_gem_object_attach_phys(obj, alignment); > >> - else if (!ret && HAS_LMEM(dev_priv)) > >> + else if (!ret && HAS_LMEM(dev_priv)) { > >> + /* > >> + * For this type of ccs buffer we need to able to read from the > >> + * CPU the clear color value found in the buffer, which might > >> + * require moving to the mappable part of lmem first, but here > >> + * we should be using dpt for this. > >> + */ > >> + WARN_ON_ONCE(intel_fb_rc_ccs_cc_plane(fb) >= 0); > > > > DPT isn't availalable on DG1. > > Hmm, does it also support the DG2_RC_CCS_CC modifier? I believe it should support I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC. > > > > >> + > >> ret = i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0); > >> + } > >> if (!ret) > >> ret = i915_gem_object_pin_pages(obj); > >> if (ret) > >> -- > >> 2.37.3 > >
diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c index 32206bd359da..8197343300ee 100644 --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c @@ -50,7 +50,18 @@ intel_pin_fb_obj_dpt(struct drm_framebuffer *fb, continue; if (HAS_LMEM(dev_priv)) { - ret = i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0); + unsigned int flags = obj->flags; + + /* + * For this type of buffer we need to able to read from the CPU + * the clear color value found in the buffer, hence we need to + * ensure it is always in the mappable part of lmem, if this is + * a small-bar device. + */ + if (intel_fb_rc_ccs_cc_plane(fb) >= 0) + flags &= ~I915_BO_ALLOC_GPU_ONLY; + ret = __i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0, + flags); if (ret) continue; } @@ -156,8 +167,17 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, ret = i915_gem_object_lock(obj, &ww); if (!ret && phys_cursor) ret = i915_gem_object_attach_phys(obj, alignment); - else if (!ret && HAS_LMEM(dev_priv)) + else if (!ret && HAS_LMEM(dev_priv)) { + /* + * For this type of ccs buffer we need to able to read from the + * CPU the clear color value found in the buffer, which might + * require moving to the mappable part of lmem first, but here + * we should be using dpt for this. + */ + WARN_ON_ONCE(intel_fb_rc_ccs_cc_plane(fb) >= 0); + ret = i915_gem_object_migrate(obj, &ww, INTEL_REGION_LMEM_0); + } if (!ret) ret = i915_gem_object_pin_pages(obj); if (ret)
For these types of display buffers, we need to able to CPU access some part of the backing memory in prepare_plane_clear_colors(). As a result we need to ensure we always place in the mappable part of lmem, which becomes necessary on small-bar systems. v2(Nirmoy & Ville): - Add some commentary for why we need to CPU access the buffer. - Split out the other changes, so we just consider the display change here. v3: - Handle this in the dpt path. Fixes: eb1c535f0d69 ("drm/i915: turn on small BAR support") Reported-by: Jianshui Yu <jianshui.yu@intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Nirmoy Das <nirmoy.das@intel.com> --- drivers/gpu/drm/i915/display/intel_fb_pin.c | 24 +++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)