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.
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
