[PATCH 5/6] drm/i915/display: add "is" member to struct intel_display

[Lucas De Marchi]

On Thu, Jun 27, 2024 at 09:47:17PM GMT, Jani Nikula wrote:
>On Thu, 27 Jun 2024, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> On Tue, Jun 18, 2024 at 05:22:55PM GMT, 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 <jani.nikula at intel.com>
>>>---
>>> .../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)
>>
>> why are we not using linux/bitmap.h that has support for bitfields with
>> multiple words and instead rolling our own?
>
>Because this is primarily about named struct members, and the bitfields
>and ORing them together is just an implementation detail.
>
>I could use bitmap_or(), but I'd have to rely on bitmap implementation
>details to get it all precisely correct. I would not be able to
>trivially use DECLARE_BITMAP() for this.
>
>Using a union can get tricky:
>
>struct intel_display_is {
>	union {
>		struct {
>			INTEL_DISPLAY_PLATFORMS(MEMBER);
>                };
>		DECLARE_BITMAP(raw, NUM_PLATFORMS);
>	};	
>};
>
>I don't know if that even works. Can't used named structs, otherwise it
>defeats the purpose.

a union like that seems good to me

Lucas De Marchi


>
>BR,
>Jani.
>
>>
>> Lucas De Marchi
>>
>>>+{
>>>+	const u8 *src = _src;
>>>+	u8 *dst = _dst;
>>>+	size_t i;
>>>+
>>>+	for (i = 0; i < size; i++)
>>>+		dst[i] |= src[i];
>>>+}
>>>+
>>>+static void merge_display_is(struct intel_display_is *dst,
>>>+			     const struct intel_display_is *src)
>>>+{
>>>+	mem_or(dst, src, sizeof(*dst));
>>>+}
>>>+
>>> 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
>>>
>
>-- 
>Jani Nikula, Intel