drm: Print erroneous hscale/vscale on failure

Submitted by Sean Paul on Sept. 5, 2018, 8:46 p.m.

Details

Message ID 20180905204610.137016-1-sean@poorly.run
State New
Headers show
Series "drm: Print erroneous hscale/vscale on failure" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Sean Paul Sept. 5, 2018, 8:46 p.m.
From: Sean Paul <seanpaul@chromium.org>

I ran across this last week when I was trying to get this function to
work. It's useful to have the scale values in the log upon failure.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/drm_atomic_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 2c23a48482da..1725546d5105 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -758,7 +758,8 @@  int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
 	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
 	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
 	if (hscale < 0 || vscale < 0) {
-		DRM_DEBUG_KMS("Invalid scaling of plane\n");
+		DRM_DEBUG_KMS("Invalid scaling of plane (%d/%d)\n",
+			      hscale, vscale);
 		drm_rect_debug_print("src: ", &plane_state->src, true);
 		drm_rect_debug_print("dst: ", &plane_state->dst, false);
 		return -ERANGE;

Comments

On Wed, Sep 05, 2018 at 04:46:01PM -0400, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> I ran across this last week when I was trying to get this function to
> work. It's useful to have the scale values in the log upon failure.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 2c23a48482da..1725546d5105 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -758,7 +758,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>  	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>  	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
>  	if (hscale < 0 || vscale < 0) {
> -		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> +		DRM_DEBUG_KMS("Invalid scaling of plane (%d/%d)\n",
> +			      hscale, vscale);
>  		drm_rect_debug_print("src: ", &plane_state->src, true);
>  		drm_rect_debug_print("dst: ", &plane_state->dst, false);
>  		return -ERANGE;
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Sep 05, 2018 at 04:46:01PM -0400, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> I ran across this last week when I was trying to get this function to
> work. It's useful to have the scale values in the log upon failure.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 2c23a48482da..1725546d5105 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -758,7 +758,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>  	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
>  	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
>  	if (hscale < 0 || vscale < 0) {
> -		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> +		DRM_DEBUG_KMS("Invalid scaling of plane (%d/%d)\n",
> +			      hscale, vscale);

I think it's just going to be -ERANGE for at least one of them.
So not quite sure what extra benefit we get from this. What
might be more helpful is printing the actual computed scale
factor and the min/max.

>  		drm_rect_debug_print("src: ", &plane_state->src, true);
>  		drm_rect_debug_print("dst: ", &plane_state->dst, false);
>  		return -ERANGE;
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Sep 06, 2018 at 02:30:24PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 05, 2018 at 04:46:01PM -0400, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > I ran across this last week when I was trying to get this function to
> > work. It's useful to have the scale values in the log upon failure.
> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 2c23a48482da..1725546d5105 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -758,7 +758,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> >  	hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >  	vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> >  	if (hscale < 0 || vscale < 0) {
> > -		DRM_DEBUG_KMS("Invalid scaling of plane\n");
> > +		DRM_DEBUG_KMS("Invalid scaling of plane (%d/%d)\n",
> > +			      hscale, vscale);
> 
> I think it's just going to be -ERANGE for at least one of them.
> So not quite sure what extra benefit we get from this. What
> might be more helpful is printing the actual computed scale
> factor and the min/max.

Good point, it will tell you which scale is invalid, but not necessarily why.
I'll tweak the args for drm_rect_calc_*scale to return -errno and pass *scale by
pointer.

v2 incoming

Sean

> 
> >  		drm_rect_debug_print("src: ", &plane_state->src, true);
> >  		drm_rect_debug_print("dst: ", &plane_state->dst, false);
> >  		return -ERANGE;
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel