On Thu, Oct 01, 2015 at 02:37:27PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 30, 2015 at 10:58:07PM +0000, Konduru, Chandra wrote:
> > > > @@ -14241,6 +14241,7 @@ static int intel_framebuffer_init(struct
> > > drm_device *dev,
> > > > {
> > > > unsigned int aligned_height;
> > > > int ret;
> > > > + int i;
> > > > u32 pitch_limit, stride_alignment;
> > > >
> > > > WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> > > > @@ -14255,7 +14256,8 @@ static int intel_framebuffer_init(struct
> > > drm_device *dev,
> > > > }
> > > > } else {
> > > > if (obj->tiling_mode == I915_TILING_X)
> > > > - mode_cmd->modifier[0] =
> > > I915_FORMAT_MOD_X_TILED;
> > > > + for (i = 0; i < drm_format_num_planes(mode_cmd-
> > > >pixel_format); i++)
> > > > + mode_cmd->modifier[i] =
> > > I915_FORMAT_MOD_X_TILED;
> > >
> > > The other branch needs updating too so that it will reject the operation
> > > if the modifier disagrees with the obj tiling mode.
> >
> > Is below something you meant?
> >
> > @@ -14223,10 +14223,12 @@ static int intel_framebuffer_init(struct
> > drm_device *d
> > 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) !=
> > - !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) {
> > - DRM_DEBUG("tiling_mode doesn't match fb
> > modifier\n");
> > - return -EINVAL;
> > + for (i = 0; i <
> > drm_format_num_planes(mode_cmd->pixel_format); i
> > + if (!!(obj->tiling_mode == I915_TILING_X) !=
> > + !!(mode_cmd->modifier[i] ==
> > I915_FORMAT_MOD_X_TILED)) {
> > + DRM_DEBUG("tiling_mode doesn't match fb
> > modifier
> > + return -EINVAL;
> > + }
> > }
>
> Yep.
>
> > } else {
> >
> > > > + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED
> > > &&
> > > > + (mode_cmd->offsets[1] & 0xFFF)) {
> > >
> > > I've been trying to solicit ideas on how we should define the offsets[];
> > > raw byte offset, or linear offset. I didn't get many opinions yet. So we
> > > need to figure it out and document it somewhere before we expose it to
> > > the world. In the meantime we could just reject non tile row aligned
> > > offsets regardless of the tiling mode.
> >
> > Above check is simply making sure tile Yf, uv offset starts on a new page.
> > Is there any issue with above check?
>
> It won't necessarily be a page boundary if we interpret offsets[] as a linear
> offset.
>
> Eg. let's assume 4x4 tile size, stride=8, and offset=16. If interpret
> the offset as a linear offset we would land at 'x', but interpreted as
> a raw byte offset (not sure that's a good name, maybe untiled offset?)
> we'd land at 'y'.
>
> -----------
> | |y |
> | | |
> |x | |
> | | |
> -----------
> |z | |
> | | |
> | | |
> | | |
> -----------
>
> If we had offset=32, then of course we would land at 'z' for both cases,
> which is why I suggested that if we haven't made up our mind about what
> offsets[] is, we could require it to be tile row aligned so that there
> would be no difference between the two interpretations.
Oh and I just figured out that linear offset would probably be better
because that also isolates us from having to think about the internal
tile layout, as in which way the bytes/owords/whatver are walked within
the tile.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
