On to, 2016-08-04 at 12:34 +0100, Chris Wilson wrote:
> On Thu, Aug 04, 2016 at 02:17:22PM +0300, Joonas Lahtinen wrote:
> >
> > >
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -2465,9 +2465,8 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
> > > return false;
> > > }
> > >
> > > - obj->tiling_mode = plane_config->tiling;
> > > - if (obj->tiling_mode == I915_TILING_X)
> > > - obj->stride = fb->pitches[0];
> > > + if (plane_config->tiling == I915_TILING_X)
> > > + obj->tiling_and_stride = fb->pitches[0] | I915_TILING_X;
> > This is not equivalent code.
> Setting tiling mode and not stride is illegal, as is setting stride for
> I915_TILING_NONE. Initial tiling_and_stride here is 0 (from object
> allocation), so the result is the same.
>
> What did I miss?
>
Patch splitting. If you mix a couple hundred line change of mechanical
changes to some functional improvements, that's really depressing to
review.
Regards, Joonas
> >
> > >
> > > } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> > > @@ -14919,15 +14918,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
> > > if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> > > /* Enforce that fb modifier and tiling mode match, but only for
> > > * X-tiled. This is needed for FBC. */
> > > - if (!!(obj->tiling_mode == I915_TILING_X) !=
> > > + if (!!(i915_gem_object_get_tiling(obj) == I915_TILING_X) !=
> > A note, !! are redundant as == returns bool already. Could remove while
> > touching.
> Petition Daniel, maybe we can now have I915_TILING_Y support here as
> well so the API is not intentionally left incomplete. Grr.
> -Chris
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
