Em Qua, 2016-09-07 às 12:05 -0400, Lyude escreveu: > 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:
We actually have both schemes, but my understanding is that firstplatform_something is more used when we actually need 2+ functions (like i9xx_crtc_enable and then ilk_crtc_enable), and we usually have something like intel_something calling firstplatform_something, or just dev_priv->something() calling the firstplatform_something vfunc. But my understanding may be wrong. Anyway, I just did the rename because skl_has_sagv() would be super confusing (returns true on KBL). I'm happy to accept suggestions here that would avoid the renaming I did. > > Reviewed-by: Lyude <[email protected]> > > 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 <[email protected]> > > --- > > 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); _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
