On Fri, 27 Jul 2018 13:38:08 -0700
Eric Anholt <eric@xxxxxxxxxx> wrote:
> Boris Brezillon <boris.brezillon@xxxxxxxxxxx> writes:
>
> > From: Eric Anholt <eric@xxxxxxxxxx>
> >
> > drm_atomic_helper_check_plane_state() takes care of checking the
> > scaling capabilities and calculating the clipped X/Y offsets for us.
> >
> > Rely on this function instead of open-coding the logic. While at it, we
> > get rid of a few fields in vc4_plane_state that can be easily extracted
> > from drm_plane_state.
> >
> > Incidentally, it seems to fix a problem we had with negative X/Y
> > positioning of YUV planes.
> >
> > Signed-off-by: Eric Anholt <eric@xxxxxxxxxx>
> > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/vc4/vc4_drv.h | 9 --
> > drivers/gpu/drm/vc4/vc4_plane.c | 210 +++++++++++++++++++++-------------------
> > 2 files changed, 111 insertions(+), 108 deletions(-)
>
> I feel like you ought to grab authorship of this patch -- you've made a
> bunch of good changes since my WIP patch.
>
> > if (num_planes > 1) {
> > - vc4_state->is_yuv = true;
> > -
> > - h_subsample = drm_format_horz_chroma_subsampling(format);
> > - v_subsample = drm_format_vert_chroma_subsampling(format);
> > - vc4_state->src_w[1] = vc4_state->src_w[0] / h_subsample;
> > - vc4_state->src_h[1] = vc4_state->src_h[0] / v_subsample;
> > + src_w /= h_subsample;
> > + src_h /= v_subsample;
>
> I'm a little nervous about leaving src_w/src_h divided after this block,
> in case we end up using them in some other calculation in a later
> change. Could we just move the divide into the vc4_get_scaling_mode()
> call?
>
> In general, I'm not enthusiastic about having src_w computations in 5
> places now. Was keeping it in the vc4 state that much of a problem?
No strong objection to keeping ->src_{h,w}[] arrays in vc4_plane_state.
>
> I'm not saying no, but copy-and-paste of these kinds of calculations are
> also a frequent source of bugs when you miss a x->y or w->h change.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
