Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

Konduru, Chandra
2015-09-10T20:59:55Z

> > > + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED
> &&
> > > + ((mode_cmd->offsets[1] / mode_cmd->pitches[1]) %
> 4)) {
> > > + DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line
> aligned\n",
> > > + mode_cmd->offsets[1]);
> > > + return -EINVAL;
> > > + }
> >
> > I was going to say I can't find anything in the spec to support this,
> > but after some more reading I got it "The display hardware requires
> > that the UV surface start satisfies four line alignment from the
> > begining of the page." So the check should be something like
> > ((offsets[1] & 0xfff) / pitches[1] % 4.

Yes, that is the bspec documentation for this check.
It is requiring the uv-offset to be 4-line aligned i.e., checking
uv-offset that is given to driver. If you take away 12-lsbs
then it is already at the page start. Why you want to take
away 12-LSBs?

> >
> > However we should anyway be able to adjust the X/Y offsets to account
> > for misalignment of offsets[1]. I think the only thing we should need
> > to check is that offsets[1] is macropixel aligned. The patch lacks such
> > a check in fact.

UV-start is with respect to buffer and not relate to any incoming flip
clipping the uv where macro-pixel adjustments are done.
Sorry, I didn't get why you think X/Y offset maco-pixel adjustments
have anything to do with uv-offset checking here in this if block?

> >
> > Feels like it would be time to expand intel_gen4_compute_page_offset()
> > to handle the new tiling formats and start using it for SKL+. Though
> > rotation may need some additional thought. Also maybe it's time to
> > dig up my old offsets[0] handling patch and refresh it a bit and try
> > to get it merged again.
>
> Just to clarify a bit. I think we can initially go with the checks in
> place, and we can work on refactoring the page offset stuff afterwards
> Actually I already started to sketch something together here :) so I
> might have a few patches to post sooner rather than later.