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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 11/10/2022 15:39, Matthew Auld wrote:
> Hi,
> 
> On 11/10/2022 14:54, Tvrtko Ursulin wrote:
>>
>> Hi Matt,
>>
>> On 04/10/2022 14:19, 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.
>>> v4(Ville):
>>>   - Drop the intel_fb_rc_ccs_cc_plane() sanity check in
>>>     pin_and_fence_fb_obj(), since we can also trigger this on DG1 it
>>>     seems.
>>>
>>> Fixes: eb1c535f0d69 ("drm/i915: turn on small BAR support")
>>
>> That one landed in 6.0 - do you want to send this (with 
>> pre-requisite(s)) to stable? Or if not do you want me to send for 6.1 
>> as part of fixes flow? In which case what are the per-requisites?
> 
> This one is only for DG2, which is still hidden behind force_probe, so 
> not too sure if it needs stable? I think the only pre-requisite is 
> 999f45620772 ("drm/i915: allow control over the flags when migrating"), 
> but again I'm not too sure how much we care about fixes for platforms 
> hidden behind force_probe? What do you think?

It is certainly not mandatory, but now that cards are about to ship and 
reach end users it may be nice to have if not too hard - at least for 
6.1 release candidates. I am not clear on the importance of the fix to 
say for sure. Like what goes bang and under what circumstances. So I do 
basically defer to someone who knows those answers.

Regards,

Tvrtko

> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>> 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 | 13 ++++++++++++-
>>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c 
>>> b/drivers/gpu/drm/i915/display/intel_fb_pin.c
>>> index 5031ee5695dd..e12339f46640 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;
>>>           }