Re: [PATCH v2] drm: rcar-du: Use dev_err_probe() to record cause of KMS init errors

2023-06-05 Thread Kieran Bingham 
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 
> ---
> 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(>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 

> 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
>

Re: [PATCH v2] drm: rcar-du: Use dev_err_probe() to record cause of KMS init errors

2023-06-04 Thread Laurent Pinchart 
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 
> ---
> 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(>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");

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] drm: rcar-du: Use dev_err_probe() to record cause of KMS init errors

2023-06-04 Thread Laurent Pinchart 
Hi Biju,

On Fri, Jun 02, 2023 at 07:29:45PM +, Biju Das wrote:
> > Subject: [PATCH v2] drm: rcar-du: Use dev_err_probe() to record cause of
> > KMS init errors
> > 
> > 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 
> > ---
> >  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(>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..a9b01027bf03 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, "failed to initialize CMM\n", ret);
>   
>   dev_err_probe(rcdu->dev, ret, "failed to initialize CMM\n");
> 
>   similarly for below one.

Oops. I wonder how I missed that, is it fails to even compile. Sorry :-(
I'll post a v2.

> > 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, "failed to initialize encoders\n", 
> > ret);
> > return ret;
> > +   }
> > 
> > if (ret == 0) {
> > dev_err(rcdu->dev, "error: no encoder could be initialized\n");

-- 
Regards,

Laurent Pinchart

RE: [PATCH v2] drm: rcar-du: Use dev_err_probe() to record cause of KMS init errors

2023-06-02 Thread Biju Das 
Hi Laurent,

Thanks for the patch.

> Subject: [PATCH v2] drm: rcar-du: Use dev_err_probe() to record cause of
> KMS init errors
> 
> 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
> 
> ---
>  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(>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..a9b01027bf03 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, "failed to initialize CMM\n", ret);

dev_err_probe(rcdu->dev, ret, "failed to initialize CMM\n");

similarly for below one.
Cheers,
Biju

>   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, "failed to initialize encoders\n",
> ret);
>   return ret;
> + }
> 
>   if (ret == 0) {
>   dev_err(rcdu->dev, "error: no encoder could be
> initialized\n");
> --
> Regards,
> 
> Laurent Pinchart