Re: [Intel-gfx] [PATCH 2/6] drm/i915/uc: mark structure passed to checker functions as const

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 28 Feb 2020, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
> On 2/28/20 1:18 AM, Jani Nikula wrote:
>> On Thu, 27 Feb 2020, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote:
>>> Follow-up patches will pass const objects from debugfs to some those
>>> functions, so we need to be ready.
>>>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: John Harrison <John.C.Harrison@Intel.com>
>>> Cc: Matthew Brost <matthew.brost@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/gt/intel_gt.h             |  6 +++---
>>>   drivers/gpu/drm/i915/gt/uc/intel_guc.h         | 10 +++++-----
>>>   drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h      |  2 +-
>>>   .../gpu/drm/i915/gt/uc/intel_guc_submission.h  |  6 +++---
>>>   drivers/gpu/drm/i915/gt/uc/intel_huc.h         |  8 ++++----
>>>   drivers/gpu/drm/i915/gt/uc/intel_uc.h          |  2 +-
>>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h       | 18 +++++++++---------
>>>   7 files changed, 26 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
>>> index 4fac043750aa..f9fbe645478d 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h
>>> @@ -18,17 +18,17 @@ struct drm_i915_private;
>>>   		  ##__VA_ARGS__);					\
>>>   } while (0)
>>>   
>>> -static inline struct intel_gt *uc_to_gt(struct intel_uc *uc)
>>> +static inline struct intel_gt *uc_to_gt(const struct intel_uc *uc)
>>>   {
>>>   	return container_of(uc, struct intel_gt, uc);
>>>   }
>>>   
>>> -static inline struct intel_gt *guc_to_gt(struct intel_guc *guc)
>>> +static inline struct intel_gt *guc_to_gt(const struct intel_guc *guc)
>>>   {
>>>   	return container_of(guc, struct intel_gt, uc.guc);
>>>   }
>>>   
>>> -static inline struct intel_gt *huc_to_gt(struct intel_huc *huc)
>>> +static inline struct intel_gt *huc_to_gt(const struct intel_huc *huc)
>>>   {
>>>   	return container_of(huc, struct intel_gt, uc.huc);
>>>   }
>> 
>> Not fond of the fact that these cast the const away. If you can return
>> const also, fine, but const in, non-const out is not fine.
>> 
>
> fair point. We usually use those functions for non-const->non-const 
> conversions, but in debugfs the objects are marked as const hence why 
> the need to add it here (the output in that case can also be marked as 
> const).
>
> What's the favorite alternative, add a guc_to_gt_const() variant, do a 
> straight container_of in the debugfs function or simply avoid marking 
> the objects as const to begin with, even if they're treated as such?

In a later patch, this seems wrong to me:

 +void intel_guc_load_status(const struct intel_guc *guc, struct drm_printer *p)
 +{
 +	struct intel_gt *gt = guc_to_gt(guc);

You get const * in, and convert it to non-const. The container_of you
suggest would do the same I believe.

How many _const() conversion macros would you get? If a lot, I'd not
bother with it.

BR,
Jani.


>
> Thanks,
> Daniele
>
>> BR,
>> Jani.
>> 
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx