Re: [Intel-gfx] [PATCH v4 4/5] drm/i915/display: consider DG2_RC_CCS_CC when migrating buffers

[Matthew Auld <matthew.auld@intel.com>]

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
>