[PATCH 4/6] drm/i915/opregion: abstract ASLE presence check

[Jani Nikula]

On Fri, 12 Jan 2024, Radhakrishna Sripada <radhakrishna.sripada at intel.com> wrote:
> On Fri, Jan 12, 2024 at 12:17:25PM +0200, Jani Nikula wrote:
>> On Thu, 11 Jan 2024, Radhakrishna Sripada <radhakrishna.sripada at intel.com> wrote:
>> > On Thu, Jan 11, 2024 at 07:21:17PM +0200, Jani Nikula wrote:
>> >> Add a function to check the opregion ASLE presence instead of accessing
>> >> the opregion structures directly.
>> >> 
>> >> Reorder the checks in i915_has_asle() to avoid the function call if
>> >> possible.
>> >> 
>> >> Cc: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
>> >> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_display_irq.c | 6 +++---
>> >>  drivers/gpu/drm/i915/display/intel_opregion.c    | 5 +++++
>> >>  drivers/gpu/drm/i915/display/intel_opregion.h    | 6 ++++++
>> >>  3 files changed, 14 insertions(+), 3 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
>> >> index 99843883cef7..f846c5b108b5 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
>> >> @@ -266,12 +266,12 @@ void i915_disable_pipestat(struct drm_i915_private *dev_priv,
>> >>  	intel_uncore_posting_read(&dev_priv->uncore, reg);
>> >>  }
>> >>  
>> >> -static bool i915_has_asle(struct drm_i915_private *dev_priv)
>> >> +static bool i915_has_asle(struct drm_i915_private *i915)
>> > Why not move this to intel_opregion.c and export it instead of
>> > intel_opregion_asle_present ?
>> 
>> I'm trying to be conscious of the possible performance impact of making
>> calls from the irq code just to find there's nothing to do.
> Makes sense.
>
>> 
>> >>  {
>> >> -	if (!dev_priv->display.opregion.asle)
>> >> +	if (!IS_PINEVIEW(i915) && !IS_MOBILE(i915))
>> > Can we extend this check to dgfx as well?
>> 
>> Extend how? This will return early for everything after IVB.
> The name of the function is bit misleading as looking at Opregion code
> and the spec beyond IVB, asle aka Mailbox 3 is present, just that it is
> not used for reading pipestat. It is used to store rvda from where VBT is read.
> Extension is not required for this purpose. Might want to clear that unless
> I misunderstood the purpose, either way 

The new function intel_opregion_asle_present() added in this patch is
exactly about whether asle mbox is present.

i915_has_asle() may be ill-named, but frankly I'm not sure what it
should be called, and it probably should not be renamed in this patch?

BR,
Jani.

>
> Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
>> 
>> BR,
>> Jani.
>> 
>> >
>> > -Radhakrishna(RK) Sripada
>> >
>> >>  		return false;
>> >>  
>> >> -	return IS_PINEVIEW(dev_priv) || IS_MOBILE(dev_priv);
>> >> +	return intel_opregion_asle_present(i915);
>> >>  }
>> >>  
>> >>  /**
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
>> >> index 8b9e820971cb..26aacb01f9ec 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
>> >> @@ -632,6 +632,11 @@ static void asle_work(struct work_struct *work)
>> >>  	asle->aslc = aslc_stat;
>> >>  }
>> >>  
>> >> +bool intel_opregion_asle_present(struct drm_i915_private *i915)
>> >> +{
>> >> +	return i915->display.opregion.asle;
>> >> +}
>> >> +
>> >>  void intel_opregion_asle_intr(struct drm_i915_private *dev_priv)
>> >>  {
>> >>  	if (dev_priv->display.opregion.asle)
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h
>> >> index 9efadfb72584..d084b30e8703 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
>> >> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
>> >> @@ -69,6 +69,7 @@ void intel_opregion_resume(struct drm_i915_private *dev_priv);
>> >>  void intel_opregion_suspend(struct drm_i915_private *dev_priv,
>> >>  			    pci_power_t state);
>> >>  
>> >> +bool intel_opregion_asle_present(struct drm_i915_private *i915);
>> >>  void intel_opregion_asle_intr(struct drm_i915_private *dev_priv);
>> >>  int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>> >>  				  bool enable);
>> >> @@ -111,6 +112,11 @@ static inline void intel_opregion_suspend(struct drm_i915_private *dev_priv,
>> >>  {
>> >>  }
>> >>  
>> >> +static inline bool intel_opregion_asle_present(struct drm_i915_private *i915)
>> >> +{
>> >> +	return false;
>> >> +}
>> >> +
>> >>  static inline void intel_opregion_asle_intr(struct drm_i915_private *dev_priv)
>> >>  {
>> >>  }
>> >> -- 
>> >> 2.39.2
>> >> 
>> 
>> -- 
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel