Re: [PATCH 06/20] drm/i915: Convert primary plane 16.16 values to regular ints

Daniel Vetter
2015-04-07T08:41:54Z

On Thu, Apr 02, 2015 at 04:03:22PM -0700, Matt Roper wrote:
> On Wed, Apr 01, 2015 at 07:59:35PM -0700, Chandra Konduru wrote:
> > This patch converts intel_plane_state->src rect from 16.16
> > values into regular ints.
> >
> > This approach aligns with sprite_plane_state->src rects
> > which are already in regular ints.
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
>
> You're not touching cursor state here, so I guess it stays in 16.16 form
> always? Does it need to be updated to behave the same way?
>
> Looking at our sprite code today, it treats intel_state->src as 16.16
> for most of the check function, then re-writes it as integer pixels near
> the end, which I guess matches the type of change you're doing here. I
> still find this pretty confusing that our structure is sometimes
> interpreted in one way and other times interpreted a different way.
>
> Personally I think it would be less error-prone if we just treated src
> as 16.16 always, but if you to keep the current logic which changes the
> meaning at the end of the check() stage, can you add some comments to
> struct intel_plane_state clarifying that?

Rewriting intel_plane_state won't work since on duplicate_state we'd need
to undo that again. That's a bit too brittle imo. I'd go with Matt's
suggestion to just use 16.16 everywhere.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation