[Intel-gfx] [PATCH] drm/i915: don't conflate is_dgfx with fake lmem

[Lucas De Marchi]

On Wed, Sep 30, 2020 at 04:58:42PM +0100, Matthew Auld wrote:
>On Wed, 30 Sep 2020 at 16:30, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>>
>> When using fake lmem for tests, we are overriding the setting in
>> device info for dgfx devices. Current users of IS_DGFX() are correct,
>> but as we add support for DG1, we are going to use it in additional
>> places to trigger dgfx-only code path.
>>
>> In future if we need we can use HAS_LMEM() instead of IS_DGFX() in the
>> places that make sense to also contemplate fake lmem use.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 45e719c79183..3f6ed142198c 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -840,9 +840,7 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>                     i915->params.fake_lmem_start) {
>>                         mkwrite_device_info(i915)->memory_regions =
>>                                 REGION_SMEM | REGION_LMEM | REGION_STOLEN;
>> -                       mkwrite_device_info(i915)->is_dgfx = true;
>>                         GEM_BUG_ON(!HAS_LMEM(i915));
>> -                       GEM_BUG_ON(!IS_DGFX(i915));
>
>Hmm, I guess this could work as an option c) but I think we will also
>need to add something like:
>
>@@ -835,7 +835,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>        u16 snb_gmch_ctl;
>
>        /* TODO: We're not aware of mappable constraints on gen8 yet */
>-       if (!IS_DGFX(i915)) {
>+       if (!IS_DGFX(i915) && !HAS_LMEM(i915)) {
>                ggtt->gmadr = pci_resource(pdev, 2);
>                ggtt->mappable_end = resource_size(&ggtt->gmadr);
>        }
>
>The point is that we need to steal the mappable aperture later(which
>is fine since it doesn't exist on "DGFX"), and use it as a substitute
>for LMEMBAR. The !mappable aperture property is also useful since it
>exercises some other parts of the code too.

humn.. this was actually the one I was on the fence switching to
HAS_LMEM().  It seemed to me we would not need to, but now I understand.

But since !HAS_LMEM() implies !IS_DGFX(), I would rather just replace
the check.

@@ -835,7 +835,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
         u16 snb_gmch_ctl;

         /* TODO: We're not aware of mappable constraints on gen8 yet */
-       if (!IS_DGFX(i915)) {
+       if (!HAS_LMEM(i915)) {
                 ggtt->gmadr = pci_resource(pdev, 2);
                 ggtt->mappable_end = resource_size(&ggtt->gmadr);
         }


Lucas De Marchi

>
>Assuming CI is happy and this unbocks you then,
>Reviewed-by: Matthew Auld <matthew.auld at intel.com>