On Tue, Jun 18, 2024 at 05:22:55PM +0300, Jani Nikula wrote:
> Facilitate using display->is.HASWELL etc. for identifying platforms and
> subplatforms. Merge platform and subplatform members together.
>
> Signed-off-by: Jani Nikula <[email protected]>
> ---
> .../gpu/drm/i915/display/intel_display_core.h | 3 +++
> .../drm/i915/display/intel_display_device.c | 19 +++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h
> b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 7715fc329057..35bea92893af 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -286,6 +286,9 @@ struct intel_display {
> /* drm device backpointer */
> struct drm_device *drm;
>
> + /* Platform identification */
> + struct intel_display_is is;
> +
> /* Display functions */
> struct {
> /* Top level crtc-ish functions */
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c
> b/drivers/gpu/drm/i915/display/intel_display_device.c
> index 0c275d85bd30..954caea38005 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> @@ -1269,8 +1269,25 @@ find_subplatform_desc(struct pci_dev *pdev, const
> struct platform_desc *desc)
> return NULL;
> }
>
> +static void mem_or(void *_dst, const void *_src, size_t size)
> +{
> + const u8 *src = _src;
> + u8 *dst = _dst;
> + size_t i;
> +
> + for (i = 0; i < size; i++)
> + dst[i] |= src[i];
I confess that here I got a bit lost. But I believe it is just a matter of
adding a few comments in the code or perhaps adjusting function names...
If my coffee is working well still, what we are doing here is ensuring that:
is.HASWELL returns true regardless the subplatform this is coming from...
like is.HASWELL_ULT or is.HASWELL_ULX.
But since you are only doing dst |= src and not doing src |= dst and also
not calling this function for different subplatforms, then individually
is.HASWELL_ULT is false for ULX platform and vice-versa.
Perhaps the name 'merge' is not a good one?
> +}
> +
> +static void merge_display_is(struct intel_display_is *dst,
> + const struct intel_display_is *src)
> +{
> + mem_or(dst, src, sizeof(*dst));
and/or perhaps we don't need this extra indirection here?
> +}
> +
> void intel_display_device_probe(struct drm_i915_private *i915)
> {
> + struct intel_display *display = &i915->display;
> struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> const struct intel_display_device_info *info;
> struct intel_display_ip_ver ip_ver = {};
> @@ -1308,11 +1325,13 @@ void intel_display_device_probe(struct
> drm_i915_private *i915)
>
> drm_WARN_ON(&i915->drm, !desc->platform || !desc->name);
> DISPLAY_RUNTIME_INFO(i915)->platform = desc->platform;
> + display->is = desc->is;
>
> subdesc = find_subplatform_desc(pdev, desc);
> if (subdesc) {
> drm_WARN_ON(&i915->drm, !subdesc->subplatform ||
> !subdesc->name);
> DISPLAY_RUNTIME_INFO(i915)->subplatform = subdesc->subplatform;
> + merge_display_is(&display->is, &subdesc->is);
> }
>
> if (ip_ver.ver || ip_ver.rel || ip_ver.step)
> --
> 2.39.2
>
