On Thu, 27 Jun 2024, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote:
> 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
With the platform enum removed in patch 6, where do we get the number of
platforms and subplatforms?
BR,
Jani.
>
> 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
--
Jani Nikula, Intel
