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, archived
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
Gustavo Sousa Nov. 5, 2024, 1:56 p.m. UTC | #3
Quoting Gustavo Sousa (2024-10-22 08:03:39-03:00)
>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?

Jani, I'll send a v2 soon-ish. I'll go ahead and assume the anwser for
the above is the former. Please stop me if otherwise :-)

--
Gustavo Sousa

>
>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
Jani Nikula Nov. 6, 2024, 9:25 a.m. UTC | #4
On Tue, 05 Nov 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Quoting Gustavo Sousa (2024-10-22 08:03:39-03:00)
>>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?
>
> Jani, I'll send a v2 soon-ish. I'll go ahead and assume the anwser for
> the above is the former. Please stop me if otherwise :-)

Sorry, inbox overflowing. I think I meant that adding HAS_DMC_WAKELOCK()
is a non-functional change.

BR,
Jani.

>
> --
> Gustavo Sousa
>
>>
>>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
Gustavo Sousa Nov. 6, 2024, 1:24 p.m. UTC | #5
Quoting Jani Nikula (2024-11-06 06:25:52-03:00)
>On Tue, 05 Nov 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> Quoting Gustavo Sousa (2024-10-22 08:03:39-03:00)
>>>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?
>>
>> Jani, I'll send a v2 soon-ish. I'll go ahead and assume the anwser for
>> the above is the former. Please stop me if otherwise :-)
>
>Sorry, inbox overflowing. I think I meant that adding HAS_DMC_WAKELOCK()
>is a non-functional change.

Ah, okay.

Well, I think the use of HAS_DMC() in the definition of
HAS_DMC_WAKELOCK() makes it a functional change when intel_dmc_wl.c uses
it (because we were not checking HAS_DMC() before). So, for an earlier
"non-functional" patch, maybe the way to go is something like the
following then?

- A patch defining HAS_DMC_WAKELOCK() with only DISPLAY_VER(i915) >=
  20 and use that macro in the DMC wakelock code.
- A modified version of this patch discarding stuff already done in the
  patch above.

Is that what you meant?

--
Gustavo Sousa

>
>BR,
>Jani.
>
>>
>> --
>> Gustavo Sousa
>>
>>>
>>>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
>
>-- 
>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);