On Mon 20 Jun 2016, Jason Ekstrand wrote:
> On Mon, Jun 20, 2016 at 3:53 PM, Chad Versace <chad.vers...@intel.com>
> wrote:
>
> > On Sat 11 Jun 2016, Jason Ekstrand wrote:
> > > ---
> > > src/intel/isl/Makefile.am | 12 ++++++++
> > > src/intel/isl/Makefile.sources | 13 +++++++--
> > > src/intel/isl/isl.c | 28 +++++++++++++++++++
> > > src/intel/isl/isl_priv.h | 24 ++++++++++++++++
> > > src/intel/isl/isl_surface_state.c | 58
> > > + case 4:
> > > + if (ISL_DEV_IS_G4X(dev)) {
> > > + isl_gen4_surf_fill_state_s(dev, state, info);
> > > + } else {
> > > + /* G45 surface state is the same as gen5 */
> > > + isl_gen5_surf_fill_state_s(dev, state, info);
> > > + }
> > > + break;
> >
> > The above can't be correct, because...
> >
> > #define GEN_IS_G4X ((GEN_VERSIONx10) == 45)
> >
> > I think you meant this:
> >
> > if (ISL_DEV_IS_G4X(dev)) {
> > /* G45 surface state is the same as gen5 */
> > isl_gen5_surf_fill_state_s(dev, state, info);
> > } else {
> > isl_gen4_surf_fill_state_s(dev, state, info);
> > }
> >
>
> You are correct. The reason it didn't trigger a bug is that the only
> difference bertween gen4 and 4.5 is that x/y offsets are introduced in
> 4.5. Since we never use ISL for filling out surfaces with x/y offsets on
> gen4.5 with this series, I never caught it. Thanks!
[snip]
> > > switch (ISL_DEV_GEN(dev)) {
> > > + case 4:
> > > + if (ISL_DEV_IS_G4X(dev)) {
> > > + isl_gen4_buffer_fill_state_s(state, info);
> > > + } else {
> > > + /* G45 surface state is the same as gen5 */
> > > + isl_gen5_buffer_fill_state_s(state, info);
> > > + }
> > > + break;
> >
> > Same as above.
> >
>
> My local fix just changes buffers to use the gen5 version for 4, 4.5, and 5
Sounds good, as buffers never use x/y offsets.
> > > -static struct isl_extent3d
> > > +static inline struct isl_extent3d
> > > get_image_alignment(const struct isl_surf *surf)
> > > {
> > > if (GEN_GEN >= 9) {
> > > @@ -199,6 +201,23 @@ isl_genX(surf_fill_state_s)(const struct isl_device
> > *dev, void *state,
> > > s.Width = info->surf->logical_level0_px.width - 1;
> > > s.Height = info->surf->logical_level0_px.height - 1;
> > >
> > > + /* In the gen6 PRM Volume 1 Part 1: Graphics Core, Section 7.18.3.7.1
> > > + * (Surface Arrays For all surfaces other than separate stencil
> > buffer):
> > > + *
> > > + * "[DevSNB] Errata: Sampler MSAA Qpitch will be 4 greater than the
> > value
> > > + * calculated in the equation above , for every other odd Surface
> > Height
> > > + * starting from 1 i.e. 1,5,9,13"
> > > + *
> > > + * Since this Qpitch errata only impacts the sampler, we have to
> > adjust the
> > > + * input for the rendering surface to achieve the same qpitch. For
> > the
> > > + * affected heights, we increment the height by 1 for the rendering
> > > + * surface.
> > > + */
> > > + if (GEN_GEN == 6 && (info->view->usage &
> > ISL_SURF_USAGE_RENDER_TARGET_BIT) &&
> > > + info->surf->samples > 1 &&
> > > + (info->surf->logical_level0_px.height % 4) == 1)
> > > + s.Height++;
> > > +
> >
> > Ouch, that's a surprising workaround.
> >
>
> Yes, yes it is. We'll have to add a similar hack to isl's gen6 layout code.
I believe isl's gen6 layout code already handles this errata, though
I could be remembering incorrectly. Specifically, grep shows that the
prm quote is already present in isl.
> > Waiting to hear your reply on the G4X comment.
With your local G4X changes,
Reviewed-by: Chad Versace <chad.vers...@intel.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
