| Message ID | 20181024100505.22436-1-boris.brezillon@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/vc4: Set PPF scaling when the source image is only vertically scaled | expand |
On Wed, 24 Oct 2018 12:05:03 +0200 Boris Brezillon <boris.brezillon@bootlin.com> wrote: > The source image might be only vertically scaled, and in this case > ->is_unity will be false, but we'd still have to force ->x_scaling[0] > to VC4_SCALING_PPF for YUV conversion to work properly. > > Let's replace the ->is_unity test by->x_scaling[0] == VC4_SCALING_NONE > to cope with that. > > Fixes: 658d8cbd07da ("drm/vc4: Fix the "no scaling" case on multi-planar YUV formats") > Cc: <stable@vger.kernel.org> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/gpu/drm/vc4/vc4_plane.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > index 60d5ad19cedd..32b7b9f47c5d 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -318,7 +318,7 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) > * even on a plane that's otherwise 1:1. Looks like only PPF > * works in that case, so let's pick that one. > */ > - if (vc4_state->is_unity) > + if (vc4_state->x_scaling[0] == VC4_SCALING_NONE) > vc4_state->x_scaling[0] = VC4_SCALING_PPF; Actually, I'm not sure about this patch is needed. According to the spec, we should not enable the scaler on the Y channel when ->is_unity is true. I tested it, and it seems to work if we just leave x_scaling[0] to VC4_SCALING_NONE. Eric, do you remember why you did that?
Boris Brezillon <boris.brezillon@bootlin.com> writes: > On Wed, 24 Oct 2018 12:05:03 +0200 > Boris Brezillon <boris.brezillon@bootlin.com> wrote: > >> The source image might be only vertically scaled, and in this case >> ->is_unity will be false, but we'd still have to force ->x_scaling[0] >> to VC4_SCALING_PPF for YUV conversion to work properly. >> >> Let's replace the ->is_unity test by->x_scaling[0] == VC4_SCALING_NONE >> to cope with that. >> >> Fixes: 658d8cbd07da ("drm/vc4: Fix the "no scaling" case on multi-planar YUV formats") >> Cc: <stable@vger.kernel.org> >> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> >> --- >> drivers/gpu/drm/vc4/vc4_plane.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c >> index 60d5ad19cedd..32b7b9f47c5d 100644 >> --- a/drivers/gpu/drm/vc4/vc4_plane.c >> +++ b/drivers/gpu/drm/vc4/vc4_plane.c >> @@ -318,7 +318,7 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) >> * even on a plane that's otherwise 1:1. Looks like only PPF >> * works in that case, so let's pick that one. >> */ >> - if (vc4_state->is_unity) >> + if (vc4_state->x_scaling[0] == VC4_SCALING_NONE) >> vc4_state->x_scaling[0] = VC4_SCALING_PPF; > > Actually, I'm not sure about this patch is needed. According to the > spec, we should not enable the scaler on the Y channel when ->is_unity > is true. > I tested it, and it seems to work if we just leave x_scaling[0] to > VC4_SCALING_NONE. Sorry, I've been delaying on this series until I could spend some time looking at the spec. I think you're right, I see no reason to kick us out of is_unity for YUV -- you can have is_unity, Y unscaled, and UV scaled.
On Wed, 07 Nov 2018 09:08:02 -0800 Eric Anholt <eric@anholt.net> wrote: > Boris Brezillon <boris.brezillon@bootlin.com> writes: > > > On Wed, 24 Oct 2018 12:05:03 +0200 > > Boris Brezillon <boris.brezillon@bootlin.com> wrote: > > > >> The source image might be only vertically scaled, and in this case > >> ->is_unity will be false, but we'd still have to force ->x_scaling[0] > >> to VC4_SCALING_PPF for YUV conversion to work properly. > >> > >> Let's replace the ->is_unity test by->x_scaling[0] == VC4_SCALING_NONE > >> to cope with that. > >> > >> Fixes: 658d8cbd07da ("drm/vc4: Fix the "no scaling" case on multi-planar YUV formats") > >> Cc: <stable@vger.kernel.org> > >> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > >> --- > >> drivers/gpu/drm/vc4/vc4_plane.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > >> index 60d5ad19cedd..32b7b9f47c5d 100644 > >> --- a/drivers/gpu/drm/vc4/vc4_plane.c > >> +++ b/drivers/gpu/drm/vc4/vc4_plane.c > >> @@ -318,7 +318,7 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) > >> * even on a plane that's otherwise 1:1. Looks like only PPF > >> * works in that case, so let's pick that one. > >> */ > >> - if (vc4_state->is_unity) > >> + if (vc4_state->x_scaling[0] == VC4_SCALING_NONE) > >> vc4_state->x_scaling[0] = VC4_SCALING_PPF; > > > > Actually, I'm not sure about this patch is needed. According to the > > spec, we should not enable the scaler on the Y channel when ->is_unity > > is true. > > I tested it, and it seems to work if we just leave x_scaling[0] to > > VC4_SCALING_NONE. > > Sorry, I've been delaying on this series until I could spend some time > looking at the spec. > > I think you're right, I see no reason to kick us out of is_unity for > YUV -- you can have is_unity, Y unscaled, and UV scaled. Okay, so I'll just remove if (vc4_state->x_scaling[0] == VC4_SCALING_NONE) vc4_state->x_scaling[0] = VC4_SCALING_PPF; What about the 2 other patches in this series? Patch 2 is fixing a real bug, patch 3 is not fixing a bug, but according to the spec PPF is better when the downscaling ratio is small.
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 60d5ad19cedd..32b7b9f47c5d 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -318,7 +318,7 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state) * even on a plane that's otherwise 1:1. Looks like only PPF * works in that case, so let's pick that one. */ - if (vc4_state->is_unity) + if (vc4_state->x_scaling[0] == VC4_SCALING_NONE) vc4_state->x_scaling[0] = VC4_SCALING_PPF; } else { vc4_state->is_yuv = false;
The source image might be only vertically scaled, and in this case ->is_unity will be false, but we'd still have to force ->x_scaling[0] to VC4_SCALING_PPF for YUV conversion to work properly. Let's replace the ->is_unity test by->x_scaling[0] == VC4_SCALING_NONE to cope with that. Fixes: 658d8cbd07da ("drm/vc4: Fix the "no scaling" case on multi-planar YUV formats") Cc: <stable@vger.kernel.org> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- drivers/gpu/drm/vc4/vc4_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)