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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.