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

From: "Konduru, Chandra" <chandra.konduru@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Vetter, Daniel" <daniel.vetter@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Syrjala, Ville" <ville.syrjala@intel.com>
Subject: Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init
Date: Thu, 10 Sep 2015 20:59:51 +0000	[thread overview]
Message-ID: <76A9B330A4D78C4D99CB292C4CC06C0E370D3CCD@fmsmsx101.amr.corp.intel.com> (raw)
In-Reply-To: <20150910194655.GF26517@intel.com>

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx