| Message ID | 20230604104958.7034-1-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | [v2] drm: rcar-du: Use dev_err_probe() to record cause of KMS init errors | expand |
This should have read v3. On Sun, Jun 04, 2023 at 01:49:58PM +0300, Laurent Pinchart wrote: > The (large) rcar_du_modeset_init() function can fail for many reasons, > two of two involving probe deferral. Use dev_err_probe() in those code > paths to record the cause of the probe deferral, in order to help > debugging probe issues. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > Change since v1: > > - Fix compilation > --- > drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c | 4 ++++ > drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 8 ++++++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > index 12a8839fe3be..5b752adb1b02 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > @@ -701,6 +701,10 @@ static int rcar_du_probe(struct platform_device *pdev) > /* DRM/KMS objects */ > ret = rcar_du_modeset_init(rcdu); > if (ret < 0) { > + /* > + * Don't use dev_err_probe(), as it would overwrite the probe > + * deferral reason recorded in rcar_du_modeset_init(). > + */ > if (ret != -EPROBE_DEFER) > dev_err(&pdev->dev, > "failed to initialize DRM/KMS (%d)\n", ret); > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > index adfb36b0e815..78b665984e35 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > @@ -932,8 +932,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) > > /* Initialize the Color Management Modules. */ > ret = rcar_du_cmm_init(rcdu); > - if (ret) > + if (ret) { > + dev_err_probe(rcdu->dev, ret, "failed to initialize CMM\n"); > return ret; > + } > > /* Create the CRTCs. */ > for (swindex = 0, hwindex = 0; swindex < rcdu->num_crtcs; ++hwindex) { > @@ -952,8 +954,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) > > /* Initialize the encoders. */ > ret = rcar_du_encoders_init(rcdu); > - if (ret < 0) > + if (ret < 0) { > + dev_err_probe(rcdu->dev, ret, "failed to initialize encoders\n"); > return ret; > + } > > if (ret == 0) { > dev_err(rcdu->dev, "error: no encoder could be initialized\n");
Quoting Laurent Pinchart (2023-06-04 11:49:58) > The (large) rcar_du_modeset_init() function can fail for many reasons, > two of two involving probe deferral. Use dev_err_probe() in those code > paths to record the cause of the probe deferral, in order to help > debugging probe issues. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > Change since v1: > > - Fix compilation Always helps ;-) > --- > drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c | 4 ++++ > drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 8 ++++++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > index 12a8839fe3be..5b752adb1b02 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c > @@ -701,6 +701,10 @@ static int rcar_du_probe(struct platform_device *pdev) > /* DRM/KMS objects */ > ret = rcar_du_modeset_init(rcdu); > if (ret < 0) { > + /* > + * Don't use dev_err_probe(), as it would overwrite the probe > + * deferral reason recorded in rcar_du_modeset_init(). > + */ > if (ret != -EPROBE_DEFER) > dev_err(&pdev->dev, > "failed to initialize DRM/KMS (%d)\n", ret); > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > index adfb36b0e815..78b665984e35 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c > @@ -932,8 +932,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) > > /* Initialize the Color Management Modules. */ > ret = rcar_du_cmm_init(rcdu); > - if (ret) > + if (ret) { > + dev_err_probe(rcdu->dev, ret, "failed to initialize CMM\n"); This could remain a single line return statement with: return dev_err_probe(rcdu->dev, ret, "failed to initialize CMM\n"); Or if you're really concerned about 80 chars: return dev_err_probe(rcdu->dev, ret, "failed to initialize CMM\n"); which is still one line cheaper than adding braces to the if statement! Anyway, either way is functional so: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > return ret; > + } > > /* Create the CRTCs. */ > for (swindex = 0, hwindex = 0; swindex < rcdu->num_crtcs; ++hwindex) { > @@ -952,8 +954,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) > > /* Initialize the encoders. */ > ret = rcar_du_encoders_init(rcdu); > - if (ret < 0) > + if (ret < 0) { > + dev_err_probe(rcdu->dev, ret, "failed to initialize encoders\n"); (same here of course) > return ret; > + } > > if (ret == 0) { > dev_err(rcdu->dev, "error: no encoder could be initialized\n"); > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c index 12a8839fe3be..5b752adb1b02 100644 --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c @@ -701,6 +701,10 @@ static int rcar_du_probe(struct platform_device *pdev) /* DRM/KMS objects */ ret = rcar_du_modeset_init(rcdu); if (ret < 0) { + /* + * Don't use dev_err_probe(), as it would overwrite the probe + * deferral reason recorded in rcar_du_modeset_init(). + */ if (ret != -EPROBE_DEFER) dev_err(&pdev->dev, "failed to initialize DRM/KMS (%d)\n", ret); diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c index adfb36b0e815..78b665984e35 100644 --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c @@ -932,8 +932,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) /* Initialize the Color Management Modules. */ ret = rcar_du_cmm_init(rcdu); - if (ret) + if (ret) { + dev_err_probe(rcdu->dev, ret, "failed to initialize CMM\n"); return ret; + } /* Create the CRTCs. */ for (swindex = 0, hwindex = 0; swindex < rcdu->num_crtcs; ++hwindex) { @@ -952,8 +954,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) /* Initialize the encoders. */ ret = rcar_du_encoders_init(rcdu); - if (ret < 0) + if (ret < 0) { + dev_err_probe(rcdu->dev, ret, "failed to initialize encoders\n"); return ret; + } if (ret == 0) { dev_err(rcdu->dev, "error: no encoder could be initialized\n");
The (large) rcar_du_modeset_init() function can fail for many reasons, two of two involving probe deferral. Use dev_err_probe() in those code paths to record the cause of the probe deferral, in order to help debugging probe issues. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- Change since v1: - Fix compilation --- drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c | 4 ++++ drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-)