Re: [PATCH v6 2/9] drm/i915: Fold vGPU active check into inner functions

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> v5:
> - Let functions take struct drm_i915_private *. (Tvrtko)
> 
> - Fold vGPU related active check into the inner functions. (Kevin)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++-------
>  drivers/gpu/drm/i915/i915_vgpu.c    | 13 +++++++++----
>  drivers/gpu/drm/i915/i915_vgpu.h    |  4 ++--
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 4668477..6f203fa 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2732,11 +2732,9 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>  	i915_address_space_init(&ggtt->base, dev_priv);
>  	ggtt->base.total += PAGE_SIZE;
>  
> -	if (intel_vgpu_active(dev_priv)) {
> -		ret = intel_vgt_balloon(dev);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = intel_vgt_balloon(dev_priv);
> +	if (ret)
> +		return ret;
>  
>  	if (!HAS_LLC(dev))
>  		ggtt->base.mm.color_adjust = i915_gtt_color_adjust;
> @@ -2836,8 +2834,7 @@ void i915_ggtt_cleanup_hw(struct drm_device *dev)
>  	i915_gem_cleanup_stolen(dev);
>  
>  	if (drm_mm_initialized(&ggtt->base.mm)) {
> -		if (intel_vgpu_active(dev_priv))
> -			intel_vgt_deballoon();
> +		intel_vgt_deballoon(dev_priv);
>  
>  		drm_mm_takedown(&ggtt->base.mm);
>  		list_del(&ggtt->base.global_link);
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index d5a7a5e..5312816 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -101,10 +101,13 @@ static struct _balloon_info_ bl_info;
>   * This function is called to deallocate the ballooned-out graphic memory, when
>   * driver is unloaded or when ballooning fails.
>   */
> -void intel_vgt_deballoon(void)
> +void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
>  {
>  	int i;
>  
> +	if (!intel_vgpu_active(dev_priv))
> +		return;
> +
>  	DRM_DEBUG("VGT deballoon.\n");
>  
>  	for (i = 0; i < 4; i++) {
> @@ -177,9 +180,8 @@ static int vgt_balloon_space(struct drm_mm *mm,
>   * Returns:
>   * zero on success, non-zero if configuration invalid or ballooning failed
>   */
> -int intel_vgt_balloon(struct drm_device *dev)
> +int intel_vgt_balloon(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
>  	unsigned long ggtt_end = ggtt->base.start + ggtt->base.total;
>  
> @@ -187,6 +189,9 @@ int intel_vgt_balloon(struct drm_device *dev)
>  	unsigned long unmappable_base, unmappable_size, unmappable_end;
>  	int ret;
>  
> +	if (!intel_vgpu_active(dev_priv))
> +		return 0;
> +
>  	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
>  	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
>  	unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
> @@ -258,6 +263,6 @@ int intel_vgt_balloon(struct drm_device *dev)
>  
>  err:
>  	DRM_ERROR("VGT balloon fail\n");
> -	intel_vgt_deballoon();
> +	intel_vgt_deballoon(dev_priv);

This function (and any others similar to it) needs to be changed to
have a proper goto teardown path as a follow-up patch. Destructors/fini
functions are only expected to be called after a succesful
initialization, so calling it at random phase in init function is bad.

But as this patch is a step in the right direction;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
> index 07e67d5..f8917c6 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -27,7 +27,7 @@
>  #include "i915_pvinfo.h"
>  
>  extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
> -extern int intel_vgt_balloon(struct drm_device *dev);
> -extern void intel_vgt_deballoon(void);
> +extern int intel_vgt_balloon(struct drm_i915_private *dev_priv);
> +extern void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
>  
>  #endif /* _I915_VGPU_H_ */
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx