Re: [PATCH v10 1/6] drm/i915/display: Add meaningful traces for QGV point info error handling

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Apr 05, 2024 at 02:35:28PM +0300, Vinod Govindapillai wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> 
> For debug purposes we need those - error path won't flood the log,
> however there has been already numerous cases, when due to lack
> of debugs, we couldn't immediately tell what was the problem on
> customer machine, which slowed down the investigation, requiring
> to get access to target device and adding those traces manually.
> 
> v2: - Make the debug more generic and move it to intel_dram_detect
>       (Gustavo Sousa)
> v3: - Use %u for unsigned variable in debug prints (Gustavo)
> 
> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 4 +++-
>  drivers/gpu/drm/i915/soc/intel_dram.c   | 2 ++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 7f2a50b4f494..77886cc21211 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -290,8 +290,10 @@ static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
>  		struct intel_qgv_point *sp = &qi->points[i];
>  
>  		ret = intel_read_qgv_point_info(dev_priv, sp, i);
> -		if (ret)
> +		if (ret) {
> +			drm_dbg_kms(&dev_priv->drm, "Could not read QGV %d info\n", i);

s/QGV/QGV point/ would be more clear.

>  			return ret;
> +		}
>  
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "QGV %d: DCLK=%d tRP=%d tRDPRE=%d tRAS=%d tRCD=%d tRC=%d\n",
> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
> index 15492b69f698..e3287f1de774 100644
> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
> @@ -681,6 +681,8 @@ void intel_dram_detect(struct drm_i915_private *i915)
>  	if (ret)
>  		return;
>  
> +	drm_dbg_kms(&i915->drm, "Num qgv points %u\n", dram_info->num_qgv_points);
> +

This doesn't seem to belong here.

>  	drm_dbg_kms(&i915->drm, "DRAM channels: %u\n", dram_info->num_channels);
>  
>  	drm_dbg_kms(&i915->drm, "Watermark level 0 adjustment needed: %s\n",
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel