<div dir="ltr">Thanks for the review and feedback, Liviu. I will follow this for my future patches. Regards, Rahul </div> <div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Tue, Aug 12, 2025 at 4:42 PM Liviu Dudau <<a href="mailto:liviu.dudau@arm.com">liviu.dudau@arm.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, Aug 12, 2025 at 03:41:19PM +0530, Rahul Kumar wrote: > Replace all dev_err(), dev_warn(), dev_info() and DRM_ERROR/WARN/INFO() > calls in drivers/gpu/drm/arm/display/komeda/komeda_crtc.c with the > corresponding drm_err(), drm_warn(), and drm_info() helpers. > > The new drm_*() logging functions take a struct drm_device * as the > first argument. This allows the DRM core to prefix log messages with > the specific DRM device name and instance, which is essential for > distinguishing logs when multiple GPUs or display controllers are present. > > This change aligns komeda with the DRM TODO item: "Convert logging to > drm_* functions with drm_device parameter". > > Signed-off-by: Rahul Kumar <<a href="mailto:rk0006818@gmail.com" target="_blank">rk0006818@gmail.com</a>> Reviewed-by: Liviu Dudau <<a href="mailto:liviu.dudau@arm.com" target="_blank">liviu.dudau@arm.com</a>> For future patches, when you make changes, please generate them with git patch -v<version> and include a change log so that reviewers can quickly figure out what has changed between emails without having to go back and forth. Best regards, Liviu > --- > .../gpu/drm/arm/display/komeda/komeda_crtc.c | 27 +++++++++++-------- > 1 file changed, 16 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > index 2ad33559a33a..e4cc1fb34e94 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > @@ -111,6 +111,7 @@ komeda_crtc_atomic_check(struct drm_crtc *crtc, > static int > komeda_crtc_prepare(struct komeda_crtc *kcrtc) > { > + struct drm_device *drm = kcrtc-><a href="http://base.dev" rel="noreferrer" target="_blank">base.dev</a>; > struct komeda_dev *mdev = kcrtc->base.dev->dev_private; > struct komeda_pipeline *master = kcrtc->master; > struct komeda_crtc_state *kcrtc_st = to_kcrtc_st(kcrtc->base.state); > @@ -128,7 +129,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc) > > err = mdev->funcs->change_opmode(mdev, new_mode); > if (err) { > - DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,", > + drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,", > mdev->dpmode, new_mode); > goto unlock; > } > @@ -142,18 +143,18 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc) > if (new_mode != KOMEDA_MODE_DUAL_DISP) { > err = clk_set_rate(mdev->aclk, komeda_crtc_get_aclk(kcrtc_st)); > if (err) > - DRM_ERROR("failed to set aclk.\n"); > + drm_err(drm, "failed to set aclk.\n"); > err = clk_prepare_enable(mdev->aclk); > if (err) > - DRM_ERROR("failed to enable aclk.\n"); > + drm_err(drm, "failed to enable aclk.\n"); > } > > err = clk_set_rate(master->pxlclk, mode->crtc_clock * 1000); > if (err) > - DRM_ERROR("failed to set pxlclk for pipe%d\n", master->id); > + drm_err(drm, "failed to set pxlclk for pipe%d\n", master->id); > err = clk_prepare_enable(master->pxlclk); > if (err) > - DRM_ERROR("failed to enable pxl clk for pipe%d.\n", master->id); > + drm_err(drm, "failed to enable pxl clk for pipe%d.\n", master->id); > > unlock: > mutex_unlock(&mdev->lock); > @@ -164,6 +165,7 @@ komeda_crtc_prepare(struct komeda_crtc *kcrtc) > static int > komeda_crtc_unprepare(struct komeda_crtc *kcrtc) > { > + struct drm_device *drm = kcrtc-><a href="http://base.dev" rel="noreferrer" target="_blank">base.dev</a>; > struct komeda_dev *mdev = kcrtc->base.dev->dev_private; > struct komeda_pipeline *master = kcrtc->master; > u32 new_mode; > @@ -180,7 +182,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc) > > err = mdev->funcs->change_opmode(mdev, new_mode); > if (err) { > - DRM_ERROR("failed to change opmode: 0x%x -> 0x%x.\n,", > + drm_err(drm, "failed to change opmode: 0x%x -> 0x%x.\n,", > mdev->dpmode, new_mode); > goto unlock; > } > @@ -200,6 +202,7 @@ komeda_crtc_unprepare(struct komeda_crtc *kcrtc) > void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, > struct komeda_events *evts) > { > + struct drm_device *drm = kcrtc-><a href="http://base.dev" rel="noreferrer" target="_blank">base.dev</a>; > struct drm_crtc *crtc = &kcrtc->base; > u32 events = evts->pipes[kcrtc->master->id]; > > @@ -212,7 +215,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, > if (wb_conn) > drm_writeback_signal_completion(&wb_conn->base, 0); > else > - DRM_WARN("CRTC[%d]: EOW happen but no wb_connector.\n", > + drm_warn(drm, "CRTC[%d]: EOW happen but no wb_connector.\n", > drm_crtc_index(&kcrtc->base)); > } > /* will handle it together with the write back support */ > @@ -236,7 +239,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, > crtc->state->event = NULL; > drm_crtc_send_vblank_event(crtc, event); > } else { > - DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n", > + drm_warn(drm, "CRTC[%d]: FLIP happened but no pending commit.\n", > drm_crtc_index(&kcrtc->base)); > } > spin_unlock_irqrestore(&crtc->dev->event_lock, flags); > @@ -309,7 +312,7 @@ komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc, > > /* wait the flip take affect.*/ > if (wait_for_completion_timeout(flip_done, HZ) == 0) { > - DRM_ERROR("wait pipe%d flip done timeout\n", kcrtc->master->id); > + drm_err(drm, "wait pipe%d flip done timeout\n", kcrtc->master->id); > if (!input_flip_done) { > unsigned long flags; > > @@ -562,6 +565,7 @@ static const struct drm_crtc_funcs komeda_crtc_funcs = { > int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms, > struct komeda_dev *mdev) > { > + struct drm_device *drm = &kms->base; > struct komeda_crtc *crtc; > struct komeda_pipeline *master; > char str[16]; > @@ -581,7 +585,7 @@ int komeda_kms_setup_crtcs(struct komeda_kms_dev *kms, > else > sprintf(str, "None"); > > - DRM_INFO("CRTC-%d: master(pipe-%d) slave(%s).\n", > + drm_info(drm, "CRTC-%d: master(pipe-%d) slave(%s).\n", > kms->n_crtcs, master->id, str); > > kms->n_crtcs++; > @@ -613,6 +617,7 @@ static int komeda_attach_bridge(struct device *dev, > struct komeda_pipeline *pipe, > struct drm_encoder *encoder) > { > + struct drm_device *drm = encoder->dev; > struct drm_bridge *bridge; > int err; > > @@ -624,7 +629,7 @@ static int komeda_attach_bridge(struct device *dev, > > err = drm_bridge_attach(encoder, bridge, NULL, 0); > if (err) > - dev_err(dev, "bridge_attach() failed for pipe: %s\n", > + drm_err(drm, "bridge_attach() failed for pipe: %s\n", > of_node_full_name(pipe->of_node)); > > return err; > -- > 2.43.0 > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ </blockquote></div>