Re: [PATCH v2] drm/mediatek: Convert legacy DRM logging to drm_* helpers in mtk_crtc.c

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

On Tue, 2025-12-23 at 15:24 +0530, Abhishek Rajput wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> Replace DRM_ERROR() and DRM_DEBUG_DRIVER() calls in
> drivers/gpu/drm/mediatek/mtk_crtc.c with the corresponding drm__err()

drm_err()

> and drm_dbg_driver() helpers.
> 
> The drm_*() logging helpers take a struct drm_device * argument,
> allowing the DRM core to prefix log messages with the correct device
> name and instance. This is required to correctly distinguish log
> messages on systems with multiple GPUs.
> 
> This change aligns the Mediatek DRM driver with the DRM TODO item:
> "Convert logging to drm_* functions with drm_device parameter".
> 

After modification as I comment,

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> Reported-by: kernel test robot <lkp@intel.com>
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202512220515.z3QybJ8I-lkp@intel.com/
> Signed-off-by: Abhishek Rajput <abhiraj21put@gmail.com>
> ---
> v2:
> - Fix unused variable warning when CONFIG_MTK_CMDQ is disabled
> - Added Reported-by and Closes tags
> Link to v1:
> https://lore.kernel.org/all/20251217070303.689913-1-abhiraj21put@gmail.com/
>  drivers/gpu/drm/mediatek/mtk_crtc.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
> index 991cdb3d7d5f..bca2e40bca54 100644
> --- a/drivers/gpu/drm/mediatek/mtk_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
> @@ -227,11 +227,12 @@ static int mtk_crtc_ddp_clk_enable(struct mtk_crtc *mtk_crtc)
>  {
>         int ret;
>         int i;
> +       struct drm_device *dev = mtk_crtc->base.dev;

Move this before 'int ret'.

> 
>         for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
>                 ret = mtk_ddp_comp_clk_enable(mtk_crtc->ddp_comp[i]);
>                 if (ret) {
> -                       DRM_ERROR("Failed to enable clock %d: %d\n", i, ret);
> +                       drm_err(dev, "Failed to enable clock %d: %d\n", i, ret);
>                         goto err;
>                 }
>         }
> @@ -343,6 +344,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_crtc *mtk_crtc)
>         struct drm_connector *connector;
>         struct drm_encoder *encoder;
>         struct drm_connector_list_iter conn_iter;
> +       struct drm_device *dev = mtk_crtc->base.dev;
>         unsigned int width, height, vrefresh, bpc = MTK_MAX_BPC;
>         int ret;
>         int i;
> @@ -371,19 +373,19 @@ static int mtk_crtc_ddp_hw_init(struct mtk_crtc *mtk_crtc)
> 
>         ret = pm_runtime_resume_and_get(crtc->dev->dev);
>         if (ret < 0) {
> -               DRM_ERROR("Failed to enable power domain: %d\n", ret);
> +               drm_err(dev, "Failed to enable power domain: %d\n", ret);
>                 return ret;
>         }
> 
>         ret = mtk_mutex_prepare(mtk_crtc->mutex);
>         if (ret < 0) {
> -               DRM_ERROR("Failed to enable mutex clock: %d\n", ret);
> +               drm_err(dev, "Failed to enable mutex clock: %d\n", ret);
>                 goto err_pm_runtime_put;
>         }
> 
>         ret = mtk_crtc_ddp_clk_enable(mtk_crtc);
>         if (ret < 0) {
> -               DRM_ERROR("Failed to enable component clocks: %d\n", ret);
> +               drm_err(dev, "Failed to enable component clocks: %d\n", ret);
>                 goto err_mutex_unprepare;
>         }
> 
> @@ -648,10 +650,11 @@ static void mtk_crtc_ddp_irq(void *data)
>         struct mtk_drm_private *priv = crtc->dev->dev_private;
> 
>  #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +       struct drm_device *dev = mtk_crtc->base.dev;
>         if (!priv->data->shadow_register && !mtk_crtc->cmdq_client.chan)
>                 mtk_crtc_ddp_config(crtc, NULL);
>         else if (mtk_crtc->cmdq_vblank_cnt > 0 && --mtk_crtc->cmdq_vblank_cnt == 0)
> -               DRM_ERROR("mtk_crtc %d CMDQ execute command timeout!\n",
> +               drm_err(dev, "mtk_crtc %d CMDQ execute command timeout!\n",
>                           drm_crtc_index(&mtk_crtc->base));

Align to previous line.

>  #else
>         if (!priv->data->shadow_register)
> @@ -776,9 +779,10 @@ static void mtk_crtc_atomic_enable(struct drm_crtc *crtc,
>  {
>         struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
>         struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> +       struct drm_device *dev = mtk_crtc->base.dev;
>         int ret;
> 
> -       DRM_DEBUG_DRIVER("%s %d\n", __func__, crtc->base.id);
> +       drm_dbg_driver(dev, "%s %d\n", __func__, crtc->base.id);
> 
>         ret = mtk_ddp_comp_power_on(comp);
>         if (ret < 0) {
> @@ -803,9 +807,10 @@ static void mtk_crtc_atomic_disable(struct drm_crtc *crtc,
>  {
>         struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
>         struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> +       struct drm_device *dev = mtk_crtc->base.dev;
>         int i;
> 
> -       DRM_DEBUG_DRIVER("%s %d\n", __func__, crtc->base.id);
> +       drm_dbg_driver(dev, "%s %d\n", __func__, crtc->base.id);
>         if (!mtk_crtc->enabled)
>                 return;
> 
> @@ -845,10 +850,11 @@ static void mtk_crtc_atomic_begin(struct drm_crtc *crtc,
>                                                                           crtc);
>         struct mtk_crtc_state *mtk_crtc_state = to_mtk_crtc_state(crtc_state);
>         struct mtk_crtc *mtk_crtc = to_mtk_crtc(crtc);
> +       struct drm_device *dev = mtk_crtc->base.dev;
>         unsigned long flags;
> 
>         if (mtk_crtc->event && mtk_crtc_state->base.event)
> -               DRM_ERROR("new event while there is still a pending event\n");
> +               drm_err(dev, "new event while there is still a pending event\n");
> 
>         if (mtk_crtc_state->base.event) {
>                 mtk_crtc_state->base.event->pipe = drm_crtc_index(crtc);
> --
> 2.34.1
> 
>