From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: lucas.demarchi@intel.com, intel-xe@lists.freedesktop.org
Subject: Re: [Intel-xe] [PATCH 09/10] fixup! drm/xe/display: Implement display support
Date: Wed, 4 Oct 2023 10:21:30 -0400 [thread overview]
Message-ID: <ZR106iLK4XgTGgfX@intel.com> (raw)
In-Reply-To: <696d81bccaca9c3b0a29996ba12afd1f09b69400.1696343521.git.jani.nikula@intel.com>
On Tue, Oct 03, 2023 at 05:34:56PM +0300, Jani Nikula wrote:
> Let the display code decide whether display hardware is there or
> not. Remove has_display member from struct xe_device_desc, and
> enable_display from struct xe_device.
on this side I got a bit confused at the beginning, probably
because I had the old i915 confusion in my head.... I had to look
to i915_pci.c to agree that this is the right thing to do.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/xe/xe_device_types.h | 2 --
> drivers/gpu/drm/xe/xe_display.c | 51 ++++++++++++----------------
> drivers/gpu/drm/xe/xe_pci.c | 13 -------
> 3 files changed, 22 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 62e1a875980b..01ab0b3d1954 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -236,8 +236,6 @@ struct xe_device {
> u8 has_llc:1;
> /** @has_range_tlb_invalidation: Has range based TLB invalidations */
> u8 has_range_tlb_invalidation:1;
> - /** @enable_display: display enabled */
> - u8 enable_display:1;
>
> #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> struct {
> diff --git a/drivers/gpu/drm/xe/xe_display.c b/drivers/gpu/drm/xe/xe_display.c
> index 75054f78d7ae..70e7afc0a890 100644
> --- a/drivers/gpu/drm/xe/xe_display.c
> +++ b/drivers/gpu/drm/xe/xe_display.c
> @@ -57,7 +57,7 @@ static void xe_display_last_close(struct drm_device *dev)
> {
> struct xe_device *xe = to_xe_device(dev);
>
> - if (xe->info.enable_display)
> + if (has_display(xe))
> intel_fbdev_restore_mode(to_xe_device(dev));
> }
>
> @@ -138,7 +138,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy)
> {
> struct xe_device *xe = to_xe_device(dev);
>
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> intel_power_domains_cleanup(xe);
> @@ -148,7 +148,7 @@ int xe_display_init_nommio(struct xe_device *xe)
> {
> int err;
>
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return 0;
>
> /* Fake uncore lock */
> @@ -168,7 +168,7 @@ static void xe_display_fini_noirq(struct drm_device *dev, void *dummy)
> {
> struct xe_device *xe = to_xe_device(dev);
>
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> intel_display_driver_remove_noirq(xe);
> @@ -179,7 +179,7 @@ int xe_display_init_noirq(struct xe_device *xe)
> {
> int err;
>
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return 0;
>
> intel_display_driver_early_probe(xe);
> @@ -208,7 +208,7 @@ static void xe_display_fini_noaccel(struct drm_device *dev, void *dummy)
> {
> struct xe_device *xe = to_xe_device(dev);
>
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> intel_display_driver_remove_nogem(xe);
> @@ -218,7 +218,7 @@ int xe_display_init_noaccel(struct xe_device *xe)
> {
> int err;
>
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return 0;
>
> err = intel_display_driver_probe_nogem(xe);
> @@ -230,7 +230,7 @@ int xe_display_init_noaccel(struct xe_device *xe)
>
> int xe_display_init(struct xe_device *xe)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return 0;
>
> return intel_display_driver_probe(xe);
> @@ -238,7 +238,7 @@ int xe_display_init(struct xe_device *xe)
>
> void xe_display_fini(struct xe_device *xe)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> /* poll work can call into fbdev, hence clean that up afterwards */
> @@ -251,7 +251,7 @@ void xe_display_fini(struct xe_device *xe)
>
> void xe_display_register(struct xe_device *xe)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> intel_display_driver_register(xe);
> @@ -261,7 +261,7 @@ void xe_display_register(struct xe_device *xe)
>
> void xe_display_unregister(struct xe_device *xe)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> intel_unregister_dsm_handler();
> @@ -271,7 +271,7 @@ void xe_display_unregister(struct xe_device *xe)
>
> void xe_display_driver_remove(struct xe_device *xe)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> intel_display_driver_remove(xe);
> @@ -281,7 +281,7 @@ void xe_display_driver_remove(struct xe_device *xe)
>
> void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> if (master_ctl & DISPLAY_IRQ)
> @@ -290,7 +290,7 @@ void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl)
>
> void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> if (gu_misc_iir & GU_MISC_GSE)
> @@ -299,7 +299,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir)
>
> void xe_display_irq_reset(struct xe_device *xe)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> gen11_display_irq_reset(xe);
> @@ -307,7 +307,7 @@ void xe_display_irq_reset(struct xe_device *xe)
>
> void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> if (gt->info.id == XE_GT0)
> @@ -341,7 +341,7 @@ static bool suspend_to_idle(void)
> void xe_display_pm_suspend(struct xe_device *xe)
> {
> bool s2idle = suspend_to_idle();
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> /*
> @@ -370,7 +370,7 @@ void xe_display_pm_suspend(struct xe_device *xe)
> void xe_display_pm_suspend_late(struct xe_device *xe)
> {
> bool s2idle = suspend_to_idle();
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> intel_power_domains_suspend(xe, s2idle);
> @@ -380,7 +380,7 @@ void xe_display_pm_suspend_late(struct xe_device *xe)
>
> void xe_display_pm_resume_early(struct xe_device *xe)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> intel_display_power_resume_early(xe);
> @@ -390,7 +390,7 @@ void xe_display_pm_resume_early(struct xe_device *xe)
>
> void xe_display_pm_resume(struct xe_device *xe)
> {
> - if (!xe->info.enable_display)
> + if (!has_display(xe))
> return;
>
> intel_dmc_resume(xe);
> @@ -418,15 +418,8 @@ void xe_display_pm_resume(struct xe_device *xe)
>
> void xe_display_probe(struct xe_device *xe)
> {
> - if (!xe->info.enable_display)
> - goto no_display;
> -
> intel_display_device_probe(xe);
>
> - if (has_display(xe))
> - return;
> -
> -no_display:
> - xe->info.enable_display = false;
> - unset_display_features(xe);
> + if (!has_display(xe))
> + unset_display_features(xe);
> }
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 8ee430c6f8b1..cbbfe75eb2d2 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -56,7 +56,6 @@ struct xe_device_desc {
>
> u8 require_force_probe:1;
> u8 is_dgfx:1;
> - u8 has_display:1;
>
> u8 has_llc:1;
> };
> @@ -207,7 +206,6 @@ static const struct xe_device_desc tgl_desc = {
> .graphics = &graphics_xelp,
> .media = &media_xem,
> PLATFORM(XE_TIGERLAKE),
> - .has_display = true,
> .has_llc = true,
> .require_force_probe = true,
> };
> @@ -216,7 +214,6 @@ static const struct xe_device_desc rkl_desc = {
> .graphics = &graphics_xelp,
> .media = &media_xem,
> PLATFORM(XE_ROCKETLAKE),
> - .has_display = true,
> .has_llc = true,
> .require_force_probe = true,
> };
> @@ -225,7 +222,6 @@ static const struct xe_device_desc adl_s_desc = {
> .graphics = &graphics_xelp,
> .media = &media_xem,
> PLATFORM(XE_ALDERLAKE_S),
> - .has_display = true,
> .has_llc = true,
> .require_force_probe = true,
> };
> @@ -236,7 +232,6 @@ static const struct xe_device_desc adl_p_desc = {
> .graphics = &graphics_xelp,
> .media = &media_xem,
> PLATFORM(XE_ALDERLAKE_P),
> - .has_display = true,
> .has_llc = true,
> .require_force_probe = true,
> .subplatforms = (const struct xe_subplatform_desc[]) {
> @@ -249,7 +244,6 @@ static const struct xe_device_desc adl_n_desc = {
> .graphics = &graphics_xelp,
> .media = &media_xem,
> PLATFORM(XE_ALDERLAKE_N),
> - .has_display = true,
> .has_llc = true,
> .require_force_probe = true,
> };
> @@ -262,7 +256,6 @@ static const struct xe_device_desc dg1_desc = {
> .media = &media_xem,
> DGFX_FEATURES,
> PLATFORM(XE_DG1),
> - .has_display = true,
> .require_force_probe = true,
> };
>
> @@ -286,7 +279,6 @@ static const struct xe_device_desc ats_m_desc = {
> .require_force_probe = true,
>
> DG2_FEATURES,
> - .has_display = false,
> };
>
> static const struct xe_device_desc dg2_desc = {
> @@ -295,14 +287,12 @@ static const struct xe_device_desc dg2_desc = {
> .require_force_probe = true,
>
> DG2_FEATURES,
> - .has_display = true,
> };
>
> static const struct xe_device_desc pvc_desc = {
> .graphics = &graphics_xehpc,
> DGFX_FEATURES,
> PLATFORM(XE_PVC),
> - .has_display = false,
> .require_force_probe = true,
> };
>
> @@ -310,7 +300,6 @@ static const struct xe_device_desc mtl_desc = {
> /* .graphics and .media determined via GMD_ID */
> .require_force_probe = true,
> PLATFORM(XE_METEORLAKE),
> - .has_display = true,
> };
>
> static const struct xe_device_desc lnl_desc = {
> @@ -574,8 +563,6 @@ static int xe_info_init(struct xe_device *xe,
> xe->info.has_flat_ccs = graphics_desc->has_flat_ccs;
> xe->info.has_range_tlb_invalidation = graphics_desc->has_range_tlb_invalidation;
>
> - xe->info.enable_display = IS_ENABLED(CONFIG_DRM_XE_DISPLAY) &&
> - desc->has_display;
> /*
> * All platforms have at least one primary GT. Any platform with media
> * version 13 or higher has an additional dedicated media GT. And
> --
> 2.39.2
>
next prev parent reply other threads:[~2023-10-04 14:21 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-03 14:34 [Intel-xe] [PATCH 00/10] xe/display: clarify display configuration Jani Nikula
2023-10-03 14:34 ` [Intel-xe] [PATCH 01/10] fixup! drm/xe/display: Implement display support Jani Nikula
2023-10-04 14:02 ` Rodrigo Vivi
2023-10-04 14:23 ` Jani Nikula
2023-10-04 14:41 ` Rodrigo Vivi
2023-10-04 14:25 ` Jani Nikula
2023-10-03 14:34 ` [Intel-xe] [PATCH 02/10] " Jani Nikula
2023-10-04 14:04 ` Rodrigo Vivi
2023-10-03 14:34 ` [Intel-xe] [PATCH 03/10] " Jani Nikula
2023-10-04 14:04 ` Rodrigo Vivi
2023-10-03 14:34 ` [Intel-xe] [PATCH 04/10] fixup! drm/xe: Introduce Xe assert macros Jani Nikula
2023-10-04 14:05 ` Rodrigo Vivi
2023-10-03 14:34 ` [Intel-xe] [PATCH 05/10] fixup! drm/xe/display: Implement display support Jani Nikula
2023-10-04 14:05 ` Rodrigo Vivi
2023-10-03 14:34 ` [Intel-xe] [PATCH 06/10] " Jani Nikula
2023-10-04 14:05 ` Rodrigo Vivi
2023-10-04 16:19 ` Lucas De Marchi
2023-10-04 16:31 ` Jani Nikula
2023-10-03 14:34 ` [Intel-xe] [PATCH 07/10] " Jani Nikula
2023-10-04 14:06 ` Rodrigo Vivi
2023-10-03 14:34 ` [Intel-xe] [PATCH 08/10] " Jani Nikula
2023-10-04 14:23 ` Rodrigo Vivi
2023-10-04 14:37 ` Jani Nikula
2023-10-04 16:14 ` Rodrigo Vivi
2023-10-04 16:41 ` Lucas De Marchi
2023-10-04 17:11 ` Jani Nikula
2023-10-04 18:03 ` Lucas De Marchi
2023-10-03 14:34 ` [Intel-xe] [PATCH 09/10] " Jani Nikula
2023-10-04 14:21 ` Rodrigo Vivi [this message]
2023-10-04 16:28 ` Lucas De Marchi
2023-10-04 17:32 ` Jani Nikula
2023-10-03 14:34 ` [Intel-xe] [PATCH 10/10] " Jani Nikula
2023-10-04 14:09 ` Rodrigo Vivi
2023-10-03 14:48 ` [Intel-xe] ✓ CI.Patch_applied: success for xe/display: clarify display configuration Patchwork
2023-10-03 14:48 ` [Intel-xe] ✗ CI.checkpatch: warning " Patchwork
2023-10-03 14:49 ` [Intel-xe] ✓ CI.KUnit: success " Patchwork
2023-10-03 14:56 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-10-03 14:57 ` [Intel-xe] ✓ CI.Hooks: " Patchwork
2023-10-03 14:58 ` [Intel-xe] ✓ CI.checksparse: " Patchwork
2023-10-03 15:35 ` [Intel-xe] ✓ CI.BAT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZR106iLK4XgTGgfX@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
--cc=lucas.demarchi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox