On Thu, Sep 10, 2015 at 07:14:58PM +0000, Konduru, Chandra wrote:
> > > + if (obj->tiling_mode == I915_TILING_X &&
> > > + !(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) {
> >
> > Your editor still seems to mess up the indentation in these cases. Can
> > you try to fix that?
>
> If condition isn't fitting in a single line, so continued in next line like any other if condition.
> Not sure what the exact indentation issue you are observing.
Stuff should line up nicely after the opening (
if (foo &&
bar &&
zonk)
and not
if (foo &&
bar &&
zonk)
>
> >
> > > + mode_cmd->modifier[1] = I915_FORMAT_MOD_X_TILED;
> > > + }
> >
> > Hmm. This obj tiling -> modifier conversion should be a fairly generic
> > thing to do, so I suggest doing it for all planes in one place. Maybe
> > add a new function intel_fb_obj_tiling_to_modifier() or something like
> > that and loop over all the planes there.
>
> Then will merge this assignment into beginning of the function
> which does for modifier[0]. Will make a loop there.
> Is that ok?
Sure.
>
> >
> > > + if (!mode_cmd->offsets[1]) {
> > > + DRM_DEBUG("uv start offset not set\n");
> > > + return -EINVAL;
> > > + }
> >
> > Still not really happy with this check since it's either too limiting, or
> > not restrictive enough. Depending on how you look at it. Do you know
> > if the hardware gets upset if you tell it use overlapping Y and UV
> > surfaces?
>
> May be not.
> But setting NV12 UV offset from userland can be a miss. So added
> this check to help userland knowing the issue.
It only catches the 0 case though. I think if userspace would forget to
set offsets[1] it would most likely also forget handles[1], so I think
we should be fairly well covered against the most obvious userspace
fumbles.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
