[Intel-xe] [PATCH] drm/xe: Replace deprecated DRM_ERROR()

[Gustavo Sousa]

Quoting Matt Roper (2023-06-01 16:33:19-03:00)
>On Thu, Jun 01, 2023 at 04:25:55PM -0300, Gustavo Sousa wrote:
>> DRM_ERROR() has been deprecated in favor of pr_err(). However, we should
>> prefer to use xe_gt_err() or drm_err() whenever possible so we get gt-
>> or device-specific output with the error message.
>> 
>> v2:
>>   - Prefer drm_err() over pr_err(). (Matt, Jani)
>> v3:
>>   - Prefer xe_gt_err() over drm_err() when possible. (Matt)
>> 
>> Cc: Matt Roper <matthew.d.roper at intel.com>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> Cc: Haridhar Kalvala <haridhar.kalvala at intel.com>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_reg_sr.c | 7 +++++--
>>  drivers/gpu/drm/xe/xe_reg_sr.h | 3 ++-
>>  drivers/gpu/drm/xe/xe_rtp.c    | 2 +-
>>  drivers/gpu/drm/xe/xe_vm.c     | 3 ++-
>>  4 files changed, 10 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
>> index 434133444d74..8580ff38b82c 100644
>> --- a/drivers/gpu/drm/xe/xe_reg_sr.c
>> +++ b/drivers/gpu/drm/xe/xe_reg_sr.c
>> @@ -19,6 +19,7 @@
>>  #include "xe_force_wake.h"
>>  #include "xe_gt.h"
>>  #include "xe_gt_mcr.h"
>> +#include "xe_gt_printk.h"
>>  #include "xe_macros.h"
>>  #include "xe_mmio.h"
>>  #include "xe_reg_whitelist.h"
>> @@ -89,7 +90,8 @@ static void reg_sr_inc_error(struct xe_reg_sr *sr)
>>  }
>>  
>>  int xe_reg_sr_add(struct xe_reg_sr *sr,
>> -                  const struct xe_reg_sr_entry *e)
>> +                  const struct xe_reg_sr_entry *e,
>> +                  struct xe_gt *gt)
>>  {
>>          unsigned long idx = e->reg.addr;
>>          struct xe_reg_sr_entry *pentry = xa_load(&sr->xa, idx);
>> @@ -122,7 +124,8 @@ int xe_reg_sr_add(struct xe_reg_sr *sr,
>>          return 0;
>>  
>>  fail:
>> -        DRM_ERROR("Discarding save-restore reg %04lx (clear: %08x, set: %08x, masked: %s, mcr: %s): ret=%d\n",
>> +        xe_gt_err(gt,
>> +                  "discarding save-restore reg %04lx (clear: %08x, set: %08x, masked: %s, mcr: %s): ret=%d\n",
>>                    idx, e->clr_bits, e->set_bits,
>>                    str_yes_no(e->reg.masked),
>>                    str_yes_no(e->reg.mcr),
>> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.h b/drivers/gpu/drm/xe/xe_reg_sr.h
>> index 0bfe66ea29bf..c3001798d9e8 100644
>> --- a/drivers/gpu/drm/xe/xe_reg_sr.h
>> +++ b/drivers/gpu/drm/xe/xe_reg_sr.h
>> @@ -19,7 +19,8 @@ struct drm_printer;
>>  int xe_reg_sr_init(struct xe_reg_sr *sr, const char *name, struct xe_device *xe);
>>  void xe_reg_sr_dump(struct xe_reg_sr *sr, struct drm_printer *p);
>>  
>> -int xe_reg_sr_add(struct xe_reg_sr *sr, const struct xe_reg_sr_entry *e);
>> +int xe_reg_sr_add(struct xe_reg_sr *sr, const struct xe_reg_sr_entry *e,
>> +                  struct xe_gt *gt);
>>  void xe_reg_sr_apply_mmio(struct xe_reg_sr *sr, struct xe_gt *gt);
>>  void xe_reg_sr_apply_whitelist(struct xe_reg_sr *sr, u32 mmio_base,
>>                                 struct xe_gt *gt);
>> diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c
>> index 43a86358efb6..956bd39fe1a0 100644
>> --- a/drivers/gpu/drm/xe/xe_rtp.c
>> +++ b/drivers/gpu/drm/xe/xe_rtp.c
>> @@ -122,7 +122,7 @@ static void rtp_add_sr_entry(const struct xe_rtp_action *action,
>>          };
>>  
>>          sr_entry.reg.addr += mmio_base;
>> -        xe_reg_sr_add(sr, &sr_entry);
>> +        xe_reg_sr_add(sr, &sr_entry, gt);
>>  }
>>  
>>  static bool rtp_process_one_sr(const struct xe_rtp_entry_sr *entry,
>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>> index 5aa2aeb14beb..f31bfc738247 100644
>> --- a/drivers/gpu/drm/xe/xe_vm.c
>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>> @@ -7,6 +7,7 @@
>>  
>>  #include <linux/dma-fence-array.h>
>>  
>> +#include <drm/drm_print.h>
>>  #include <drm/ttm/ttm_execbuf_util.h>
>>  #include <drm/ttm/ttm_tt.h>
>>  #include <drm/xe_drm.h>
>> @@ -3056,7 +3057,7 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>>          }
>>  
>>          if (XE_IOCTL_ERR(xe, xe_vm_is_closed(vm))) {
>> -                DRM_ERROR("VM closed while we began looking up?\n");
>> +                drm_err(&xe->drm, "VM closed while we began looking up?\n");
>
>You could also use the 'dev' that was passed as a parameter to the
>function rather than re-extracting it from xe.  Up to you though.

Ah! That's right.

>
>Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

Thanks for the review! I just sent a new version with the fix and your r-b and
will apply once CI comes back clean.

--
Gustavo Sousa

>
>>                  err = -ENOENT;
>>                  goto put_vm;
>>          }
>> -- 
>> 2.40.1
>> 
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation