Re: [PATCH] drm: get rid of DRM_DEBUG_* log calls in drm core, files drm_{b,c}*.c

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 30 Dec 2021, Claudio Suarez <cssk@net-c.es> wrote:
> DRM_DEBUG_* and DRM_* log calls are deprecated.
> Change them to drm_dbg_* / drm_{err,info,...} calls in drm core
> files.
>
> To avoid making a very big patch, this change is split in
> smaller patches. This one includes drm_{b,c}*.c

Personally, I'd split it further to smaller patches.

>
> Signed-off-by: Claudio Suarez <cssk@net-c.es>
> ---
>  drivers/gpu/drm/drm_blend.c          |   6 +-
>  drivers/gpu/drm/drm_bridge.c         |   6 +-
>  drivers/gpu/drm/drm_bufs.c           | 116 +++++++++++++--------------
>  drivers/gpu/drm/drm_client_modeset.c | 109 +++++++++++++------------
>  drivers/gpu/drm/drm_color_mgmt.c     |   6 +-
>  drivers/gpu/drm/drm_connector.c      |  37 +++++----
>  drivers/gpu/drm/drm_context.c        |  18 ++---
>  drivers/gpu/drm/drm_crtc.c           |  40 ++++-----
>  drivers/gpu/drm/drm_crtc_helper.c    |  61 +++++++-------
>  9 files changed, 211 insertions(+), 188 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index ec37cbfabb50..4a988815f998 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -450,7 +450,7 @@ static int drm_atomic_helper_crtc_normalize_zpos(struct drm_crtc *crtc,
>  	int i, n = 0;
>  	int ret = 0;
>  
> -	DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
> +	drm_dbg_atomic(dev, "[CRTC:%d:%s] calculating normalized zpos values\n",
>  			 crtc->base.id, crtc->name);

Throughout the patch, please fix the indentation on the following
lines. The lines should be aligned at the next column after the opening
brace "(" in the function call, like they used to be.

>  
>  	states = kmalloc_array(total_planes, sizeof(*states), GFP_KERNEL);
> @@ -469,7 +469,7 @@ static int drm_atomic_helper_crtc_normalize_zpos(struct drm_crtc *crtc,
>  			goto done;
>  		}
>  		states[n++] = plane_state;
> -		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] processing zpos value %d\n",
> +		drm_dbg_atomic(dev, "[PLANE:%d:%s] processing zpos value %d\n",
>  				 plane->base.id, plane->name,
>  				 plane_state->zpos);
>  	}
> @@ -480,7 +480,7 @@ static int drm_atomic_helper_crtc_normalize_zpos(struct drm_crtc *crtc,
>  		plane = states[i]->plane;
>  
>  		states[i]->normalized_zpos = i;
> -		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n",
> +		drm_dbg_atomic(dev, "[PLANE:%d:%s] normalized zpos value %d\n",
>  				 plane->base.id, plane->name, i);
>  	}
>  	crtc_state->zpos_changed = true;
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index c96847fc0ebc..b108377b4b40 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -288,10 +288,12 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>  	list_del(&bridge->chain_node);
>  
>  #ifdef CONFIG_OF
> -	DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
> +	drm_err(encoder->dev,
> +		  "failed to attach bridge %pOF to encoder %s: %d\n",
>  		  bridge->of_node, encoder->name, ret);
>  #else
> -	DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
> +	drm_err(encoder->dev,
> +		  "failed to attach bridge to encoder %s: %d\n",
>  		  encoder->name, ret);
>  #endif
>  
> diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
> index fcca21e8efac..dd8e100e120c 100644
> --- a/drivers/gpu/drm/drm_bufs.c
> +++ b/drivers/gpu/drm/drm_bufs.c
> @@ -171,8 +171,8 @@ static int drm_addmap_core(struct drm_device *dev, resource_size_t offset,
>  		kfree(map);
>  		return -EINVAL;
>  	}
> -	DRM_DEBUG("offset = 0x%08llx, size = 0x%08lx, type = %d\n",
> -		  (unsigned long long)map->offset, map->size, map->type);
> +	drm_dev_dbg(dev, "offset = 0x%08llx, size = 0x%08lx, type = %d\n",
> +		    (unsigned long long)map->offset, map->size, map->type);

Please avoid drm_dev_dbg() when struct drm_device is available,
throughout the patch. DRM_DEBUG() -> drm_dbg_core() in drm_*.[ch].

Clearly dev is struct drm_device here, and in a lot of places, so you're
passing an incompatible pointer to drm_dev_dbg(), which is for when you
only have struct device available.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center