Re: [Intel-gfx] [PATCH v2 0/6] i915: Move display identification/probing under display/

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 22 May 2023, Matt Roper <matthew.d.roper@intel.com> wrote:
> Since i915's display code will soon be shared by two DRM drivers (i915
> and Xe), it makes sense for the display code itself to be responsible
> for recognizing the platform it's running on rather than relying on the
> making the top-level DRM driver handle this.  This also becomes more
> important for all platforms MTL and beyond where we're not really
> supposed to identify platform behavior by PCI device ID anymore, but
> rather by the hardware IP version reported by the device through the
> GMD_ID register.
>
> This series creates a more well-defined split between display and
> non-display deviceinfo/runtimeinfo and then moves the definition of the
> display-specific feature flags under the display/ code.  Finally, it
> switches MTL (and all future platforms), to select the display feature
> flags based on the hardware's GMD_ID identification.

My primary gripe with this series is that I didn't think of it myself.

I was always hung up on making device info (i915->__info) itself a
pointer, and got stuck there.

Nice job, and many thanks!

I see that there are already a bunch of reviews, so I didn't dig into
all the details. I left some nitpicky comments, but nothing that can't
be fixed later.

Acked-by: Jani Nikula <jani.nikula@intel.com>


BR,
Jani.

>
> v2:
>  - Move DISPLAY_INFO() definition one patch earlier.  (Andrzej)
>  - Rename display's runtime default structure to __runtime_defaults to
>    make it more clear what the purpose is.  (Andrzej)
>  - Simplify copy of runtime defaults to per-device runtime data.
>    (Andrzej)
>  - Fix uninitialized ptr use on error path during device probe. (lkp)
>  - Add extra patch moving display-specific feature test macros to
>    display/intel_display_device.h
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>
> Matt Roper (5):
>   drm/i915/display: Move display device info to header under display/
>   drm/i915: Convert INTEL_INFO()->display to a pointer
>   drm/i915/display: Move display runtime info to display structure
>   drm/i915/display: Make display responsible for probing its own IP
>   drm/i915/display: Handle GMD_ID identification in display code
>   drm/i915/display: Move feature test macros to intel_display_device.h
>
>  drivers/gpu/drm/i915/Makefile                 |   2 +
>  drivers/gpu/drm/i915/display/intel_color.c    |  31 +-
>  drivers/gpu/drm/i915/display/intel_crtc.c     |   2 +-
>  drivers/gpu/drm/i915/display/intel_cursor.c   |   4 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |   2 +-
>  drivers/gpu/drm/i915/display/intel_display.h  |  10 +-
>  .../drm/i915/display/intel_display_device.c   | 764 ++++++++++++++++++
>  .../drm/i915/display/intel_display_device.h   | 129 +++
>  .../drm/i915/display/intel_display_power.c    |   6 +-
>  .../drm/i915/display/intel_display_reg_defs.h |  14 +-
>  drivers/gpu/drm/i915/display/intel_fb_pin.c   |   2 +-
>  drivers/gpu/drm/i915/display/intel_fbc.c      |   6 +-
>  drivers/gpu/drm/i915/display/intel_hdcp.c     |   2 +-
>  drivers/gpu/drm/i915/display/intel_hti.c      |   2 +-
>  .../drm/i915/display/skl_universal_plane.c    |   2 +-
>  drivers/gpu/drm/i915/display/skl_watermark.c  |   8 +-
>  drivers/gpu/drm/i915/i915_driver.c            |  17 +-
>  drivers/gpu/drm/i915/i915_drv.h               |  65 +-
>  drivers/gpu/drm/i915/i915_pci.c               | 382 +--------
>  drivers/gpu/drm/i915/i915_reg.h               |  33 -
>  drivers/gpu/drm/i915/intel_device_info.c      | 113 +--
>  drivers/gpu/drm/i915/intel_device_info.h      |  67 +-
>  drivers/gpu/drm/i915/intel_step.c             |   8 +-
>  23 files changed, 1030 insertions(+), 641 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/display/intel_display_device.c
>  create mode 100644 drivers/gpu/drm/i915/display/intel_display_device.h

-- 
Jani Nikula, Intel Open Source Graphics Center