On Thu, 29 Jun 2023, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
> On 27/06/2023 16:14, Jani Nikula wrote:
>> Finally we can get rid of the pseudo-const write-once device info, and
>> convert it into a const pointer to device info in rodata.
>>
>> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
>> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 4 ++--
>> drivers/gpu/drm/i915/intel_device_info.c | 17 ++++-------------
>> 2 files changed, 6 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 8947d1201298..682ef2b5c7d5 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -203,7 +203,7 @@ struct drm_i915_private {
>> /* i915 device parameters */
>> struct i915_params params;
>>
>> - const struct intel_device_info __info; /* Use INTEL_INFO() to access. */
>> + const struct intel_device_info *__info; /* Use INTEL_INFO() to access. */
>> struct intel_runtime_info __runtime; /* Use RUNTIME_INFO() to access. */
>> struct intel_driver_caps caps;
>>
>> @@ -415,7 +415,7 @@ static inline struct intel_gt *to_gt(struct drm_i915_private *i915)
>> (engine__) && (engine__)->uabi_class == (class__); \
>> (engine__) = rb_to_uabi_engine(rb_next(&(engine__)->uabi_node)))
>>
>> -#define INTEL_INFO(i915) (&(i915)->__info)
>> +#define INTEL_INFO(i915) ((i915)->__info)
>> #define RUNTIME_INFO(i915) (&(i915)->__runtime)
>> #define DISPLAY_INFO(i915) ((i915)->display.info.__device_info)
>> #define DISPLAY_RUNTIME_INFO(i915) (&(i915)->display.info.__runtime_info)
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> index 0740922cd71f..ea0ec6174ce5 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -364,13 +364,6 @@ void intel_device_info_runtime_init_early(struct drm_i915_private *i915)
>> intel_device_info_subplatform_init(i915);
>> }
>>
>> -/* FIXME: Remove this, and make device info a const pointer to rodata. */
>> -static struct intel_device_info *
>> -mkwrite_device_info(struct drm_i915_private *i915)
>> -{
>> - return (struct intel_device_info *)INTEL_INFO(i915);
>> -}
>> -
>> static const struct intel_display_device_info no_display = {};
>>
>> /**
>> @@ -430,26 +423,24 @@ void intel_device_info_driver_create(struct drm_i915_private *i915,
>> u16 device_id,
>> const struct intel_device_info *match_info)
>> {
>> - struct intel_device_info *info;
>> struct intel_runtime_info *runtime;
>> u16 ver, rel, step;
>>
>> - /* Setup the write-once "constant" device info */
>> - info = mkwrite_device_info(i915);
>> - memcpy(info, match_info, sizeof(*info));
>> + /* Setup INTEL_INFO() */
>> + i915->__info = match_info;
>>
>> /* Initialize initial runtime info from static const data and pdev. */
>> runtime = RUNTIME_INFO(i915);
>> memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime));
>>
>> /* Probe display support */
>> - i915->display.info.__device_info = intel_display_device_probe(i915, info->has_gmd_id,
>> + i915->display.info.__device_info = intel_display_device_probe(i915, HAS_GMD_ID(i915),
>
> Why does intel_display_device_probe need an explicit has_gmdid when it
> could use HAS_GMD_ID itself?
I think all of the display related initialization from this function
should be moved within intel_display_device_probe() in follow-up.
> But anyway, LGTM. If it saves you some time before the other time zone
> comes online:
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Thanks,
Jani.
>
> Regards,
>
> Tvrtko
>
>> &ver, &rel, &step);
>> memcpy(DISPLAY_RUNTIME_INFO(i915),
>> &DISPLAY_INFO(i915)->__runtime_defaults,
>> sizeof(*DISPLAY_RUNTIME_INFO(i915)));
>>
>> - if (info->has_gmd_id) {
>> + if (HAS_GMD_ID(i915)) {
>> DISPLAY_RUNTIME_INFO(i915)->ip.ver = ver;
>> DISPLAY_RUNTIME_INFO(i915)->ip.rel = rel;
>> DISPLAY_RUNTIME_INFO(i915)->ip.step = step;
--
Jani Nikula, Intel Open Source Graphics Center
