On Sun, 25 Jan 2026, Atharva Tiwari <[email protected]> wrote:

[Quoting this part from the cover letter]
> A side effect of this patch is that the iGPU becomes enabled on iMacs.
> However, iMacs can’t use the iGPU for rendering (They can't link-train
> the internal display), so displays must be disabled on iMacs.

Rendering and display are quite separate in the hardware and in the
driver. Perhaps you do not mean "rendering" here? Perhaps it can be used
for rendering but not display?

> Disable display on iMacs, as they can't do link training
> on the internal display.

Okay, so perhaps there's no eDP connected. But what about the other
connectors on the iGPU? What about everything else in the display
hardware?

If you can figure out that it's specifically link training that fails (a
dmesg would be useful to show this) there clearly is display hardware,
right?

There's at least three levels where this could be handled, depending on
details:

- Display probe (the patch at hand). Assumes there's no display
  hardware, at all. The driver doesn't touch the hardware, which
  continues to consume power, it's not put in low power states. Not
  optimal if there actually is display hardware.

- Display disable. See intel_display_device_enabled(). The driver takes
  over the hardware, puts it to sleep, but prevents all connectors from
  being connected.

- eDP disable. Add a quirk somewhere to enforce eDP is disconnected, but
  other connectors can be used.

> (tested on iMac20,1)

It would be quite useful to indicate the PCI ID of the device in
question.

>
> Signed-off-by: Atharva Tiwari <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_display_device.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c 
> b/drivers/gpu/drm/i915/display/intel_display_device.c
> index 1170afaa8680..3fb47232e7a4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> @@ -3,6 +3,7 @@
>   * Copyright © 2023 Intel Corporation
>   */
>  
> +#include <linux/dmi.h>
>  #include <linux/pci.h>
>  
>  #include <drm/drm_color_mgmt.h>
> @@ -1657,6 +1658,7 @@ struct intel_display *intel_display_device_probe(struct 
> pci_dev *pdev,
>       const struct platform_desc *desc;
>       const struct subplatform_desc *subdesc;
>       enum intel_step step;
> +     const char *product_name = dmi_get_system_info(DMI_PRODUCT_NAME);
>  
>       display = kzalloc(sizeof(*display), GFP_KERNEL);
>       if (!display)
> @@ -1674,6 +1676,11 @@ struct intel_display 
> *intel_display_device_probe(struct pci_dev *pdev,
>               goto no_display;
>       }
>  
> +     if (dmi_match(DMI_BOARD_VENDOR, "Apple Inc.") && !strncmp(product_name, 
> "iMac", 4)) {
> +             drm_dbg_kms(display->drm, "iMac Detected, Disabling display\n");
> +             goto no_display;
> +     }
> +

This function is high level code, its clarity is important, and low
level device specific quirks do not belong here. They are a
distraction. At a minimum this needs to be hidden away in a function
that gets called from has_no_display() or something.

BR,
Jani.

>       desc = find_platform_desc(pdev);
>       if (!desc) {
>               drm_dbg_kms(display->drm,

-- 
Jani Nikula, Intel

Reply via email to