My only thought is that it seems like we prefix functions skl_, kbl_,
etc. just to indicate which generation introduced the feature. Skl uses
quite a few sandybridge and haswell functions. If this is a little
closer to what most intel devs would expect the naming to be though
then:
Reviewed-by: Lyude <cpaul@xxxxxxxxxx>
going through the other patches now as well
On Tue, 2016-09-06 at 21:52 -0300, Paulo Zanoni wrote:
> The plan is to introduce intel_has_sagv() and then use it to discover
> which platforms actually support it.
>
> I thought about keeping the functions with their current skl names,
> but found two problems: (i) skl_has_sagv() would become a very
> confusing name, and (ii) intel_atomic_commit_tail() doesn't seem to
> be
> calling any functions whose name start with a platform name, so the
> "intel_" naming scheme seems make more sense than the "firstplatorm_"
> naming scheme here.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 10 +++++-----
> drivers/gpu/drm/i915/intel_display.c | 8 ++++----
> drivers/gpu/drm/i915/intel_drv.h | 6 +++---
> drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++-------------
> 4 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 053a347..503c69d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1972,11 +1972,11 @@ struct drm_i915_private {
> struct vlv_s0ix_state vlv_s0ix_state;
>
> enum {
> - I915_SKL_SAGV_UNKNOWN = 0,
> - I915_SKL_SAGV_DISABLED,
> - I915_SKL_SAGV_ENABLED,
> - I915_SKL_SAGV_NOT_CONTROLLED
> - } skl_sagv_status;
> + I915_SAGV_UNKNOWN = 0,
> + I915_SAGV_DISABLED,
> + I915_SAGV_ENABLED,
> + I915_SAGV_NOT_CONTROLLED
> + } sagv_status;
>
> struct {
> /*
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 6b4d7ac..4dd4961 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14379,8 +14379,8 @@ static void intel_atomic_commit_tail(struct
> drm_atomic_state *state)
> * SKL workaround: bspec recommends we disable the
> SAGV when we
> * have more then one pipe enabled
> */
> - if (IS_SKYLAKE(dev_priv) &&
> !skl_can_enable_sagv(state))
> - skl_disable_sagv(dev_priv);
> + if (IS_SKYLAKE(dev_priv) &&
> !intel_can_enable_sagv(state))
> + intel_disable_sagv(dev_priv);
>
> intel_modeset_verify_disabled(dev);
> }
> @@ -14438,8 +14438,8 @@ static void intel_atomic_commit_tail(struct
> drm_atomic_state *state)
> }
>
> if (IS_SKYLAKE(dev_priv) && intel_state->modeset &&
> - skl_can_enable_sagv(state))
> - skl_enable_sagv(dev_priv);
> + intel_can_enable_sagv(state))
> + intel_enable_sagv(dev_priv);
>
> drm_atomic_helper_commit_hw_done(state);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index d084c1b..bb55b61 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1741,9 +1741,9 @@ void ilk_wm_get_hw_state(struct drm_device
> *dev);
> void skl_wm_get_hw_state(struct drm_device *dev);
> void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> struct skl_ddb_allocation *ddb /* out */);
> -bool skl_can_enable_sagv(struct drm_atomic_state *state);
> -int skl_enable_sagv(struct drm_i915_private *dev_priv);
> -int skl_disable_sagv(struct drm_i915_private *dev_priv);
> +bool intel_can_enable_sagv(struct drm_atomic_state *state);
> +int intel_enable_sagv(struct drm_i915_private *dev_priv);
> +int intel_disable_sagv(struct drm_i915_private *dev_priv);
> bool skl_ddb_allocation_equals(const struct skl_ddb_allocation *old,
> const struct skl_ddb_allocation *new,
> enum pipe pipe);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 4f833a0..32588e3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2896,12 +2896,12 @@ skl_wm_plane_id(const struct intel_plane
> *plane)
> * - We're not using an interlaced display configuration
> */
> int
> -skl_enable_sagv(struct drm_i915_private *dev_priv)
> +intel_enable_sagv(struct drm_i915_private *dev_priv)
> {
> int ret;
>
> - if (dev_priv->skl_sagv_status ==
> I915_SKL_SAGV_NOT_CONTROLLED ||
> - dev_priv->skl_sagv_status == I915_SKL_SAGV_ENABLED)
> + if (dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED ||
> + dev_priv->sagv_status == I915_SAGV_ENABLED)
> return 0;
>
> DRM_DEBUG_KMS("Enabling the SAGV\n");
> @@ -2919,19 +2919,19 @@ skl_enable_sagv(struct drm_i915_private
> *dev_priv)
> */
> if (ret == -ENXIO) {
> DRM_DEBUG_DRIVER("No SAGV found on system,
> ignoring\n");
> - dev_priv->skl_sagv_status =
> I915_SKL_SAGV_NOT_CONTROLLED;
> + dev_priv->sagv_status = I915_SAGV_NOT_CONTROLLED;
> return 0;
> } else if (ret < 0) {
> DRM_ERROR("Failed to enable the SAGV\n");
> return ret;
> }
>
> - dev_priv->skl_sagv_status = I915_SKL_SAGV_ENABLED;
> + dev_priv->sagv_status = I915_SAGV_ENABLED;
> return 0;
> }
>
> static int
> -skl_do_sagv_disable(struct drm_i915_private *dev_priv)
> +intel_do_sagv_disable(struct drm_i915_private *dev_priv)
> {
> int ret;
> uint32_t temp = GEN9_SAGV_DISABLE;
> @@ -2945,19 +2945,19 @@ skl_do_sagv_disable(struct drm_i915_private
> *dev_priv)
> }
>
> int
> -skl_disable_sagv(struct drm_i915_private *dev_priv)
> +intel_disable_sagv(struct drm_i915_private *dev_priv)
> {
> int ret, result;
>
> - if (dev_priv->skl_sagv_status ==
> I915_SKL_SAGV_NOT_CONTROLLED ||
> - dev_priv->skl_sagv_status == I915_SKL_SAGV_DISABLED)
> + if (dev_priv->sagv_status == I915_SAGV_NOT_CONTROLLED ||
> + dev_priv->sagv_status == I915_SAGV_DISABLED)
> return 0;
>
> DRM_DEBUG_KMS("Disabling the SAGV\n");
> mutex_lock(&dev_priv->rps.hw_lock);
>
> /* bspec says to keep retrying for at least 1 ms */
> - ret = wait_for(result = skl_do_sagv_disable(dev_priv), 1);
> + ret = wait_for(result = intel_do_sagv_disable(dev_priv), 1);
> mutex_unlock(&dev_priv->rps.hw_lock);
>
> if (ret == -ETIMEDOUT) {
> @@ -2971,18 +2971,18 @@ skl_disable_sagv(struct drm_i915_private
> *dev_priv)
> */
> if (result == -ENXIO) {
> DRM_DEBUG_DRIVER("No SAGV found on system,
> ignoring\n");
> - dev_priv->skl_sagv_status =
> I915_SKL_SAGV_NOT_CONTROLLED;
> + dev_priv->sagv_status = I915_SAGV_NOT_CONTROLLED;
> return 0;
> } else if (result < 0) {
> DRM_ERROR("Failed to disable the SAGV\n");
> return result;
> }
>
> - dev_priv->skl_sagv_status = I915_SKL_SAGV_DISABLED;
> + dev_priv->sagv_status = I915_SAGV_DISABLED;
> return 0;
> }
>
> -bool skl_can_enable_sagv(struct drm_atomic_state *state)
> +bool intel_can_enable_sagv(struct drm_atomic_state *state)
> {
> struct drm_device *dev = state->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
--
Cheers,
Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
