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. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx