On Mon, 22 May 2023, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote:
> +static const struct intel_display_device_info xe_lpdp_display = {
> + XE_LPD_FEATURES,
> + .has_cdclk_crawl = 1,
> + .has_cdclk_squash = 1,
> +
> + .__runtime_defaults.ip.ver = 14,
> + .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A) | BIT(INTEL_FBC_B),
> + .__runtime_defaults.cpu_transcoder_mask =
> + BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
> + BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
> +};
> +
> +static const struct pci_device_id intel_display_ids[] = {
Since this is not used for MODULE_DEVICE_TABLE(), there's no requirement
for the array to be struct pci_device_id.
You can either have a struct with compatible members, or just undef and
redefine INTEL_VGA_DEVICE() to initialize a struct with pci id and const
struct intel_display_device_info *, so you can avoid the cast in
intel_display_device_probe().
See what's done with subplatform init arrays in intel_device_info.c.
This too is nitpicking, and can be fixed later.
> + INTEL_I830_IDS(&i830_display),
> + INTEL_I845G_IDS(&i845_display),
> + INTEL_I85X_IDS(&i85x_display),
> + INTEL_I865G_IDS(&i865g_display),
> + INTEL_I915G_IDS(&i915g_display),
> + INTEL_I915GM_IDS(&i915gm_display),
> + INTEL_I945G_IDS(&i945g_display),
> + INTEL_I945GM_IDS(&i945gm_display),
> + INTEL_I965G_IDS(&i965g_display),
> + INTEL_G33_IDS(&g33_display),
> + INTEL_I965GM_IDS(&i965gm_display),
> + INTEL_GM45_IDS(&gm45_display),
> + INTEL_G45_IDS(&g45_display),
> + INTEL_PINEVIEW_G_IDS(&g33_display),
> + INTEL_PINEVIEW_M_IDS(&g33_display),
> + INTEL_IRONLAKE_D_IDS(&ilk_d_display),
> + INTEL_IRONLAKE_M_IDS(&ilk_m_display),
> + INTEL_SNB_D_IDS(&snb_display),
> + INTEL_SNB_M_IDS(&snb_display),
> + INTEL_IVB_Q_IDS(NULL), /* must be first IVB in list */
> + INTEL_IVB_M_IDS(&ivb_display),
> + INTEL_IVB_D_IDS(&ivb_display),
> + INTEL_HSW_IDS(&hsw_display),
> + INTEL_VLV_IDS(&vlv_display),
> + INTEL_BDW_IDS(&bdw_display),
> + INTEL_CHV_IDS(&chv_display),
> + INTEL_SKL_IDS(&skl_display),
> + INTEL_BXT_IDS(&bxt_display),
> + INTEL_GLK_IDS(&glk_display),
> + INTEL_KBL_IDS(&skl_display),
> + INTEL_CFL_IDS(&skl_display),
> + INTEL_ICL_11_IDS(&gen11_display),
> + INTEL_EHL_IDS(&gen11_display),
> + INTEL_JSL_IDS(&gen11_display),
> + INTEL_TGL_12_IDS(&tgl_display),
> + INTEL_DG1_IDS(&tgl_display),
> + INTEL_RKL_IDS(&rkl_display),
> + INTEL_ADLS_IDS(&adl_s_display),
> + INTEL_RPLS_IDS(&adl_s_display),
> + INTEL_ADLP_IDS(&xe_lpd_display),
> + INTEL_ADLN_IDS(&xe_lpd_display),
> + INTEL_RPLP_IDS(&xe_lpd_display),
> + INTEL_DG2_IDS(&xe_hpd_display),
> +
> + /* FIXME: Replace this with a GMD_ID lookup */
> + INTEL_MTL_IDS(&xe_lpdp_display),
> +};
> +
> +const struct intel_display_device_info *
> +intel_display_device_probe(u16 pci_devid)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(intel_display_ids); i++) {
> + if (intel_display_ids[i].device == pci_devid)
> + return (struct intel_display_device_info *)intel_display_ids[i].driver_data;
> + }
> +
I wonder if a debug message here would be helpful. *shrug*.
> + return &no_display;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 4d158927c78b..e1507ae59f2d 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -574,7 +574,6 @@ void intel_device_info_driver_create(struct drm_i915_private *i915,
> {
> struct intel_device_info *info;
> struct intel_runtime_info *runtime;
> - struct intel_display_runtime_info *display_runtime;
>
> /* Setup the write-once "constant" device info */
> info = mkwrite_device_info(i915);
> @@ -583,9 +582,10 @@ void intel_device_info_driver_create(struct drm_i915_private *i915,
> /* Initialize initial runtime info from static const data and pdev. */
> runtime = RUNTIME_INFO(i915);
> memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime));
> - display_runtime = DISPLAY_RUNTIME_INFO(i915);
> - memcpy(display_runtime, &DISPLAY_INFO(i915)->__runtime_defaults,
> - sizeof(*display_runtime));
> +
> + /* Probe display support */
> + info->display = intel_display_device_probe(device_id);
> + *DISPLAY_RUNTIME_INFO(i915) = DISPLAY_INFO(i915)->__runtime_defaults;
I think I'd keep the memcpy.
Acked-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>
> runtime->device_id = device_id;
> }
--
Jani Nikula, Intel Open Source Graphics Center
