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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 27, 2024 at 09:47:17PM GMT, Jani Nikula wrote:
On Thu, 27 Jun 2024, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> 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@xxxxxxxxx>
---
.../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



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux