| Message ID | f31933222f44e4a9224e41399a96896eb243e653.1687878757.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: further device info fixes and cleanups | expand |
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@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > 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? But anyway, LGTM. If it saves you some time before the other time zone comes online: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> 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;
On Thu, 29 Jun 2023, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> 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@intel.com> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> 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@intel.com> 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;
On Thu, 29 Jun 2023, Jani Nikula <jani.nikula@intel.com> wrote: > On Thu, 29 Jun 2023, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> 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@intel.com> >>> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >>> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >>> --- >>> 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@intel.com> And pushed the series to drm-intel-next, thanks for the reviews everyone! BR, Jani. > > 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;
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), &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;
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@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_device_info.c | 17 ++++------------- 2 files changed, 6 insertions(+), 15 deletions(-)