Re: [PATCH v4 12/12] drm/xe/vram: Report VRAM errors using structured xe_log API

[Dnyaneshwar Bhadane <dnyaneshwar.bhadane@Intel.com>]

On 30-Sep-26 12:18 PM, Mallesh, Koujalagi wrote:
> 
> On 28-09-2026 02:51 pm, Dnyaneshwar Bhadane wrote:
>> Replace drm_err()/drm_info() with xe_log_err_fatal()/xe_log_info()
>> using the VRAM component tag. The "no CPU visible VRAM" error uses
>> tile or device as the log location depending on the caller context.
>>
>> Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_vram.c | 20 ++++++++++++++------
>>   1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_vram.c b/drivers/gpu/drm/xe/xe_vram.c
>> index fa06c7ae56b1..6c8558042ad4 100644
>> --- a/drivers/gpu/drm/xe/xe_vram.c
>> +++ b/drivers/gpu/drm/xe/xe_vram.c
>> @@ -17,6 +17,7 @@
>>   #include "xe_device.h"
>>   #include "xe_force_wake.h"
>>   #include "xe_gt_mcr.h"
>> +#include "xe_log.h"
>>   #include "xe_map.h"
>>   #include "xe_migrate.h"
>>   #include "xe_mmio.h"
>> @@ -46,7 +47,8 @@ static int determine_lmem_bar_size(struct xe_device 
>> *xe, struct xe_vram_region *
>>       struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>       if (!resource_is_valid(pdev, LMEM_BAR)) {
>> -        drm_err(&xe->drm, "pci resource is not valid\n");
>> +        xe_log_err_fatal(xe, VRAM, -ENXIO,
>> +                 "pci resource is not valid\n");
> Message should start with small or capital letter, make it consistency 
> across all logs.
>>           return -ENXIO;
>>       }
>> @@ -254,8 +256,13 @@ static int vram_region_init(struct xe_device *xe, 
>> struct xe_vram_region *vram,
>>       vram->io_size = min_t(u64, usable_size, remain_io_size);
>>       if (!vram->io_size) {
>> -        drm_err(&xe->drm, "%s without any CPU visible VRAM. Aborting. 
>> \n",
>> -            tile ? "Tile" : "Device");
>> +        if (tile) {
>> +            xe_log_err_fatal(tile, VRAM, -ENODEV,
>> +                     "Tile without any CPU visible VRAM. Aborting.\n");
>> +        } else {
>> +            xe_log_err_fatal(xe, VRAM, -ENODEV,
>> +                     "Device without any CPU visible VRAM. Aborting. 
>> \n");
>> +        }
> hmm, we can change it like:
> 
> xe_log_err_fatal(tile ? tile : xe, VRAM, -ENODEV, "%s without any CPU 
Hi,
No we can't use ternary operator in first argument of 
xe_log_err_fatal(). It must be the be the static know pointer type.

Alternative is that we could use the like

i.e[1] xe_log_err_fatal(xe, VRAM, -ENODEV, "%s without any CPU visible 
VRAM. Aborting.\n", tile ? "Tile" : "Device");

This way we lose location for the Tile/Device but it is conveyed through 
the messages string prefix based on run time condition evaluation on the 
print.
> visible VRAM. Aborting.\n",
>           tile ? "Tile" : "Device");
> 
> right?
> 
>>           return -ENODEV;
>>       }
>> @@ -320,7 +327,7 @@ int xe_vram_probe(struct xe_device *xe)
>>       err = determine_lmem_bar_size(xe, &lmem_bar);
>>       if (err)
>>           return err;
>> -    drm_info(&xe->drm, "VISIBLE VRAM: %pa, %pa\n", 
>> &lmem_bar.io_start, &lmem_bar.io_size);
>> +    xe_log_info(xe, VRAM, "VISIBLE VRAM: %pa, %pa\n", 
>> &lmem_bar.io_start, &lmem_bar.io_size);
>>       remain_io_size = lmem_bar.io_size;
>> @@ -342,8 +349,9 @@ int xe_vram_probe(struct xe_device *xe)
>>               return err;
>>           if (total_size > lmem_bar.io_size) {
>> -            drm_info(&xe->drm, "VRAM: %pa is larger than resource 
>> %pa\n",
>> -                 &total_size, &lmem_bar.io_size);
>> +            xe_log_info(xe, VRAM,
>> +                    "Total size %pa is larger than resource %pa\n",
>> +                    &total_size, &lmem_bar.io_size);
>>           }
>>           remain_io_size -= min_t(u64, tile->mem.vram- 
>> >actual_physical_size, remain_io_size);