[Intel-gfx] [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

Konduru, Chandra chandra.konduru at intel.com
Thu Sep 10 12:14:58 PDT 2015


> > +		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.

> 
> > +			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?

> 
> > +		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.



More information about the Intel-gfx mailing list