Long story short, we'll need to identify platforms in display code using
some other way than i915 core IS_<PLATFORM>() macros if we ever want to
make a strict separation between the display and non-display parts.
I tossed some ideas around (see the bottom of this mail), Lucas liked
something similar to what I have here. Essentially with this, you can
replace platform checks like IS_TIGERLAKE(i915) with
display->is.TIGERLAKE and subplatform checks like IS_RAPTORLAKE_S(i915)
with display->is.ALDERLAKE_S_RAPTORLAKE_S.
It would be possible to drop the ALDERLAKE_S or "parent" platform part
there, but I think it's useful in many cases to be explicit it's a
subplatform we're talking about.
It's also possible to convert all of this to lowercase if desired,
i.e. display->is.tigerlake and display->is.alderlake_s_raptorlake_s.
There's one more wrinkle I didn't address; currently IS_HASWELL_ULT()
and IS_BROADWELL_ULT() also match the ULX variants. This is not the case
here yet.
Thoughts?
BR,
Jani.
void foo(void)
{
/*
* Examples with a platform check (Tigerlake) and a subplatform check
* (Alderlake S subplatform Raptorlake S).
*/
/*
* is_<platform>(display). Same as i915 core, but lowercase.
*
* Pros:
* - Easy to convert
* - Short
*
* Cons:
* - Need to keep defining new macros for new platfoms and subplatforms
*/
if (is_tigerlake(display) || is_alderlake_s_raptorlake_s(display)) {
}
/*
* is_platform(display, <platform>) check.
*
* Alternatively is_plat() or is_display() or something else?
*
* Pros:
* - Can be made to handle both platforms and subplatforms by
* renumbering subplatforms enum
* - No need to define new macros for new platforms
*
* Cons:
* - A bit long
*/
if (is_platform(display, INTEL_DISPLAY_TIGERLAKE) ||
is_platform(display, INTEL_DISPLAY_ALDERLAKE_S_RAPTORLAKE_S)) {
}
/*
* An is_platform() with a macro wrapper to abbreviate param.
*
* Pros:
* - Shorter
*
* Cons:
* - Throws off cscope and gnu global
*/
if (is_platform(display, TIGERLAKE) ||
is_platform(display, ALDERLAKE_S_RAPTORLAKE_S)) {
}
/*
* Functions to return platform/subplatforms.
*
* Pros:
* - No need to define new macros for new platforms
*
* Cons:
* - Long
* - Need separate checks for platform/subplatform
*/
if (intel_platform(display) == INTEL_DISPLAY_TIGERLAKE ||
intel_subplatform(display) == INTEL_DISPLAY_ALDERLAKE_S_RAPTORLAKE_S) {
}
/*
* Initialize bitfields in display according to platform/subplatform
*
* Pros:
* - Really short
* - Does not pollute namespace with is_something
*
* Cons:
* - Pollutes top level struct intel_display
* - Kind of belongs in display device or runtime info, but that would
* again be too long to be helpful
*/
if (display->is_tigerlake || display->alderlake_s_raptorlake_s) {
}
}
Jani Nikula (6):
drm/i915/display: use a macro to initialize subplatforms
drm/i915/display: use a macro to define platform enumerations
drm/i915/display: join the platform and subplatform macros
drm/i915/display: add "display is" structure with platform members
drm/i915/display: add "is" member to struct intel_display
drm/i915/display: remove the display platform enum as unnecessary
.../gpu/drm/i915/display/intel_display_core.h | 3 +
.../drm/i915/display/intel_display_device.c | 79 ++++++---
.../drm/i915/display/intel_display_device.h | 165 +++++++++---------
3 files changed, 136 insertions(+), 111 deletions(-)
--
2.39.2
