diff mbox series

[11/13] drm/i915/dmc_wl: Add and use HAS_DMC_WAKELOCK()

Message ID 20241021222744.294371-12-gustavo.sousa@intel.com (mailing list archive)
State New
Headers show
Series drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD | expand

Commit Message

Gustavo Sousa Oct. 21, 2024, 10:27 p.m. UTC 
In order to be able to use the DMC wakelock, we also need to know that
the display hardware has support for DMC, which is a runtime info.
Define HAS_DMC_WAKELOCK(), which checks for both DMC availability and IP
version, and use it in place of directly checking the display version.

Since we depend on runtime info, also make sure to call
intel_dmc_wl_init() only after we have probed the hardware for such info
(i.e. after intel_display_device_info_runtime_init()).

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_device.h | 1 +
 drivers/gpu/drm/i915/display/intel_display_driver.c | 2 +-
 drivers/gpu/drm/i915/display/intel_dmc_wl.c         | 4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)

Comments

Jani Nikula Oct. 22, 2024, 9:37 a.m. UTC | #1 
On Mon, 21 Oct 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> In order to be able to use the DMC wakelock, we also need to know that
> the display hardware has support for DMC, which is a runtime info.
> Define HAS_DMC_WAKELOCK(), which checks for both DMC availability and IP
> version, and use it in place of directly checking the display version.
>
> Since we depend on runtime info, also make sure to call
> intel_dmc_wl_init() only after we have probed the hardware for such info
> (i.e. after intel_display_device_info_runtime_init()).

Non-functional changes combined with functional changes. Please split.

BR,
Jani.

>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_device.h | 1 +
>  drivers/gpu/drm/i915/display/intel_display_driver.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_dmc_wl.c         | 4 ++--
>  3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 071a36b51f79..5f78fd127fe0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -128,6 +128,7 @@ enum intel_display_subplatform {
>  #define HAS_DDI(i915)			(DISPLAY_INFO(i915)->has_ddi)
>  #define HAS_DISPLAY(i915)		(DISPLAY_RUNTIME_INFO(i915)->pipe_mask != 0)
>  #define HAS_DMC(i915)			(DISPLAY_RUNTIME_INFO(i915)->has_dmc)
> +#define HAS_DMC_WAKELOCK(i915)		(HAS_DMC(i915) && DISPLAY_VER(i915) >= 20)
>  #define HAS_DOUBLE_BUFFERED_M_N(i915)	(DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915))
>  #define HAS_DP_MST(i915)		(DISPLAY_INFO(i915)->has_dp_mst)
>  #define HAS_DP20(i915)			(IS_DG2(i915) || DISPLAY_VER(i915) >= 14)
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 673f9b965494..8afaa9cb89d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -200,7 +200,6 @@ void intel_display_driver_early_probe(struct drm_i915_private *i915)
>  	intel_dpll_init_clock_hook(i915);
>  	intel_init_display_hooks(i915);
>  	intel_fdi_init_hook(i915);
> -	intel_dmc_wl_init(&i915->display);
>  }
>  
>  /* part #1: call before irq install */
> @@ -238,6 +237,7 @@ int intel_display_driver_probe_noirq(struct drm_i915_private *i915)
>  		return 0;
>  
>  	intel_dmc_init(display);
> +	intel_dmc_wl_init(display);
>  
>  	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
>  	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> index 8283b607aac4..f6ec79b0e39d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> @@ -250,7 +250,7 @@ static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address)
>  
>  static bool __intel_dmc_wl_supported(struct intel_display *display)
>  {
> -	if (DISPLAY_VER(display) < 20 ||
> +	if (!HAS_DMC_WAKELOCK(display) ||
>  	    !intel_dmc_has_payload(display) ||
>  	    !display->params.enable_dmc_wl)
>  		return false;
> @@ -263,7 +263,7 @@ void intel_dmc_wl_init(struct intel_display *display)
>  	struct intel_dmc_wl *wl = &display->wl;
>  
>  	/* don't call __intel_dmc_wl_supported(), DMC is not loaded yet */
> -	if (DISPLAY_VER(display) < 20 || !display->params.enable_dmc_wl)
> +	if (!HAS_DMC_WAKELOCK(display) || !display->params.enable_dmc_wl)
>  		return;
>  
>  	INIT_DELAYED_WORK(&wl->work, intel_dmc_wl_work);
Gustavo Sousa Oct. 22, 2024, 11:03 a.m. UTC | #2 
Quoting Jani Nikula (2024-10-22 06:37:51-03:00)
>On Mon, 21 Oct 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> In order to be able to use the DMC wakelock, we also need to know that
>> the display hardware has support for DMC, which is a runtime info.
>> Define HAS_DMC_WAKELOCK(), which checks for both DMC availability and IP
>> version, and use it in place of directly checking the display version.
>>
>> Since we depend on runtime info, also make sure to call
>> intel_dmc_wl_init() only after we have probed the hardware for such info
>> (i.e. after intel_display_device_info_runtime_init()).
>
>Non-functional changes combined with functional changes. Please split.

Do you mean changing the call site of intel_dmc_wl_init() as being
non-functional? Or is it something else?

If this is about the former, I would argue that's not really
non-functional, because we are changing the order of how things are
done... But if making that a standalone patch is preferred, I can do
that.

--
Gustavo Sousa

>
>BR,
>Jani.
>
>>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display_device.h | 1 +
>>  drivers/gpu/drm/i915/display/intel_display_driver.c | 2 +-
>>  drivers/gpu/drm/i915/display/intel_dmc_wl.c         | 4 ++--
>>  3 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
>> index 071a36b51f79..5f78fd127fe0 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>> @@ -128,6 +128,7 @@ enum intel_display_subplatform {
>>  #define HAS_DDI(i915)                        (DISPLAY_INFO(i915)->has_ddi)
>>  #define HAS_DISPLAY(i915)                (DISPLAY_RUNTIME_INFO(i915)->pipe_mask != 0)
>>  #define HAS_DMC(i915)                        (DISPLAY_RUNTIME_INFO(i915)->has_dmc)
>> +#define HAS_DMC_WAKELOCK(i915)                (HAS_DMC(i915) && DISPLAY_VER(i915) >= 20)
>>  #define HAS_DOUBLE_BUFFERED_M_N(i915)        (DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915))
>>  #define HAS_DP_MST(i915)                (DISPLAY_INFO(i915)->has_dp_mst)
>>  #define HAS_DP20(i915)                        (IS_DG2(i915) || DISPLAY_VER(i915) >= 14)
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
>> index 673f9b965494..8afaa9cb89d2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
>> @@ -200,7 +200,6 @@ void intel_display_driver_early_probe(struct drm_i915_private *i915)
>>          intel_dpll_init_clock_hook(i915);
>>          intel_init_display_hooks(i915);
>>          intel_fdi_init_hook(i915);
>> -        intel_dmc_wl_init(&i915->display);
>>  }
>>  
>>  /* part #1: call before irq install */
>> @@ -238,6 +237,7 @@ int intel_display_driver_probe_noirq(struct drm_i915_private *i915)
>>                  return 0;
>>  
>>          intel_dmc_init(display);
>> +        intel_dmc_wl_init(display);
>>  
>>          i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
>>          i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
>> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
>> index 8283b607aac4..f6ec79b0e39d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
>> @@ -250,7 +250,7 @@ static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address)
>>  
>>  static bool __intel_dmc_wl_supported(struct intel_display *display)
>>  {
>> -        if (DISPLAY_VER(display) < 20 ||
>> +        if (!HAS_DMC_WAKELOCK(display) ||
>>              !intel_dmc_has_payload(display) ||
>>              !display->params.enable_dmc_wl)
>>                  return false;
>> @@ -263,7 +263,7 @@ void intel_dmc_wl_init(struct intel_display *display)
>>          struct intel_dmc_wl *wl = &display->wl;
>>  
>>          /* don't call __intel_dmc_wl_supported(), DMC is not loaded yet */
>> -        if (DISPLAY_VER(display) < 20 || !display->params.enable_dmc_wl)
>> +        if (!HAS_DMC_WAKELOCK(display) || !display->params.enable_dmc_wl)
>>                  return;
>>  
>>          INIT_DELAYED_WORK(&wl->work, intel_dmc_wl_work);
>
>-- 
>Jani Nikula, Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 071a36b51f79..5f78fd127fe0 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -128,6 +128,7 @@  enum intel_display_subplatform {
 #define HAS_DDI(i915)			(DISPLAY_INFO(i915)->has_ddi)
 #define HAS_DISPLAY(i915)		(DISPLAY_RUNTIME_INFO(i915)->pipe_mask != 0)
 #define HAS_DMC(i915)			(DISPLAY_RUNTIME_INFO(i915)->has_dmc)
+#define HAS_DMC_WAKELOCK(i915)		(HAS_DMC(i915) && DISPLAY_VER(i915) >= 20)
 #define HAS_DOUBLE_BUFFERED_M_N(i915)	(DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915))
 #define HAS_DP_MST(i915)		(DISPLAY_INFO(i915)->has_dp_mst)
 #define HAS_DP20(i915)			(IS_DG2(i915) || DISPLAY_VER(i915) >= 14)
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 673f9b965494..8afaa9cb89d2 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -200,7 +200,6 @@  void intel_display_driver_early_probe(struct drm_i915_private *i915)
 	intel_dpll_init_clock_hook(i915);
 	intel_init_display_hooks(i915);
 	intel_fdi_init_hook(i915);
-	intel_dmc_wl_init(&i915->display);
 }
 
 /* part #1: call before irq install */
@@ -238,6 +237,7 @@  int intel_display_driver_probe_noirq(struct drm_i915_private *i915)
 		return 0;
 
 	intel_dmc_init(display);
+	intel_dmc_wl_init(display);
 
 	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
 	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index 8283b607aac4..f6ec79b0e39d 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -250,7 +250,7 @@  static bool intel_dmc_wl_check_range(struct intel_display *display, u32 address)
 
 static bool __intel_dmc_wl_supported(struct intel_display *display)
 {
-	if (DISPLAY_VER(display) < 20 ||
+	if (!HAS_DMC_WAKELOCK(display) ||
 	    !intel_dmc_has_payload(display) ||
 	    !display->params.enable_dmc_wl)
 		return false;
@@ -263,7 +263,7 @@  void intel_dmc_wl_init(struct intel_display *display)
 	struct intel_dmc_wl *wl = &display->wl;
 
 	/* don't call __intel_dmc_wl_supported(), DMC is not loaded yet */
-	if (DISPLAY_VER(display) < 20 || !display->params.enable_dmc_wl)
+	if (!HAS_DMC_WAKELOCK(display) || !display->params.enable_dmc_wl)
 		return;
 
 	INIT_DELAYED_WORK(&wl->work, intel_dmc_wl_work);