| Message ID | 1429543368-5279-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 20, 2015 at 04:22:48PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Add Y tiling support to skl_do_mmio_flip. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Damien Lespiau <damien.lespiau@intel.com> > Cc: Sonika Jindal <sonika.jindal@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > Looks like we completely missed this completely parallel instance of plane > programming code when working on Y tiling support! > > Will rotation need supporting here as well? Oh dear this is a bit ugly. We definitely need either this for 4.1 or take out the Y-tiling support for 4.1. Rotation only made 4.2, so I think we're good, but we do indeed need to take rotation into account here too. And it seems like we also need a pile of igt testcases to make sure Y-tiling, rotation (even 180 probably) works correctly with pageflips. Yay for kms_flip growing even more. > And what about the comment at intel_do_mmio_flip saying this was only > temporary code? Well once we have atomic this code will disappear, but it sounds like that's still a bit off unfortunately. -Daniel
On 04/20/2015 05:22 PM, Daniel Vetter wrote: > On Mon, Apr 20, 2015 at 04:22:48PM +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Add Y tiling support to skl_do_mmio_flip. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Damien Lespiau <damien.lespiau@intel.com> >> Cc: Sonika Jindal <sonika.jindal@intel.com> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> --- >> Looks like we completely missed this completely parallel instance of plane >> programming code when working on Y tiling support! >> >> Will rotation need supporting here as well? > > Oh dear this is a bit ugly. We definitely need either this for 4.1 or take > out the Y-tiling support for 4.1. Rotation only made 4.2, so I think > we're good, but we do indeed need to take rotation into account here too. What is ugly, this patch or the overall situation? > And it seems like we also need a pile of igt testcases to make sure > Y-tiling, rotation (even 180 probably) works correctly with pageflips. Yay > for kms_flip growing even more. I added Y to kms_flip_tiling back then - but both that one and kms_flip don't do any pipe crc. So not much use in automated fashion. And testdisplay apparently does not hit this path. I did not have working crc hw back then, maybe that has changed, I'll check it and then, depending on the outcome, see if I can add something automated. Either way, adding this fix sounds better than reverting everything. Regards, Tvrtko
On Mon, Apr 20, 2015 at 05:38:20PM +0100, Tvrtko Ursulin wrote: > > On 04/20/2015 05:22 PM, Daniel Vetter wrote: > >On Mon, Apr 20, 2015 at 04:22:48PM +0100, Tvrtko Ursulin wrote: > >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> > >>Add Y tiling support to skl_do_mmio_flip. > >> > >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>Cc: Damien Lespiau <damien.lespiau@intel.com> > >>Cc: Sonika Jindal <sonika.jindal@intel.com> > >>Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >>--- > >>Looks like we completely missed this completely parallel instance of plane > >>programming code when working on Y tiling support! > >> > >>Will rotation need supporting here as well? > > > >Oh dear this is a bit ugly. We definitely need either this for 4.1 or take > >out the Y-tiling support for 4.1. Rotation only made 4.2, so I think > >we're good, but we do indeed need to take rotation into account here too. > > What is ugly, this patch or the overall situation? The situation, since catching this kind of stuff is why we should have a real userspace implementation ;-) > >And it seems like we also need a pile of igt testcases to make sure > >Y-tiling, rotation (even 180 probably) works correctly with pageflips. Yay > >for kms_flip growing even more. > > I added Y to kms_flip_tiling back then - but both that one and kms_flip > don't do any pipe crc. So not much use in automated fashion. And testdisplay > apparently does not hit this path. > > I did not have working crc hw back then, maybe that has changed, I'll check > it and then, depending on the outcome, see if I can add something automated. Hm right we'd need a real crc based testcase, not just exercising the codepaths. Simplest solution might be to extend the rotation test with a new subtests which does a pageflip before sampling the crc. Adding crc checks to the kms_flip monster is probably not a good idea. > Either way, adding this fix sounds better than reverting everything. I don't mean to revert everything, just disable the feature in case we can't fix it up properly. -Daniel
On Mon, Apr 20, 2015 at 05:38:20PM +0100, Tvrtko Ursulin wrote: > > On 04/20/2015 05:22 PM, Daniel Vetter wrote: > >On Mon, Apr 20, 2015 at 04:22:48PM +0100, Tvrtko Ursulin wrote: > >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> > >>Add Y tiling support to skl_do_mmio_flip. > >> > >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>Cc: Damien Lespiau <damien.lespiau@intel.com> > >>Cc: Sonika Jindal <sonika.jindal@intel.com> > >>Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >>--- > >>Looks like we completely missed this completely parallel instance of plane > >>programming code when working on Y tiling support! > >> > >>Will rotation need supporting here as well? > > > >Oh dear this is a bit ugly. We definitely need either this for 4.1 or take > >out the Y-tiling support for 4.1. Rotation only made 4.2, so I think > >we're good, but we do indeed need to take rotation into account here too. > > What is ugly, this patch or the overall situation? > > >And it seems like we also need a pile of igt testcases to make sure > >Y-tiling, rotation (even 180 probably) works correctly with pageflips. Yay > >for kms_flip growing even more. > > I added Y to kms_flip_tiling back then - but both that one and kms_flip > don't do any pipe crc. So not much use in automated fashion. And testdisplay > apparently does not hit this path. > > I did not have working crc hw back then, maybe that has changed, I'll check > it and then, depending on the outcome, see if I can add something automated. > > Either way, adding this fix sounds better than reverting everything. On 2nd thought skl is still tagged as preliminary, so we don't have any issues here at all. Queued for -next, thanks for the patch. While looking at skl_do_mmio_flip I noticed though that rotation support seems to be completely absent. Or did I miss something? -Daniel
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6236
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB 318/318 318/318
IVB 341/341 341/341
BYT 287/287 287/287
BDW 318/318 318/318
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
Note: You need to pay more attention to line start with '*'
On 4/20/2015 11:14 PM, Daniel Vetter wrote: > On Mon, Apr 20, 2015 at 05:38:20PM +0100, Tvrtko Ursulin wrote: >> >> On 04/20/2015 05:22 PM, Daniel Vetter wrote: >>> On Mon, Apr 20, 2015 at 04:22:48PM +0100, Tvrtko Ursulin wrote: >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> >>>> Add Y tiling support to skl_do_mmio_flip. >>>> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> Cc: Damien Lespiau <damien.lespiau@intel.com> >>>> Cc: Sonika Jindal <sonika.jindal@intel.com> >>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>>> --- >>>> Looks like we completely missed this completely parallel instance of plane >>>> programming code when working on Y tiling support! >>>> >>>> Will rotation need supporting here as well? >>> >>> Oh dear this is a bit ugly. We definitely need either this for 4.1 or take >>> out the Y-tiling support for 4.1. Rotation only made 4.2, so I think >>> we're good, but we do indeed need to take rotation into account here too. >> >> What is ugly, this patch or the overall situation? >> >>> And it seems like we also need a pile of igt testcases to make sure >>> Y-tiling, rotation (even 180 probably) works correctly with pageflips. Yay >>> for kms_flip growing even more. >> >> I added Y to kms_flip_tiling back then - but both that one and kms_flip >> don't do any pipe crc. So not much use in automated fashion. And testdisplay >> apparently does not hit this path. >> >> I did not have working crc hw back then, maybe that has changed, I'll check >> it and then, depending on the outcome, see if I can add something automated. >> >> Either way, adding this fix sounds better than reverting everything. > > On 2nd thought skl is still tagged as preliminary, so we don't have any > issues here at all. Queued for -next, thanks for the patch. > > While looking at skl_do_mmio_flip I noticed though that rotation support > seems to be completely absent. Or did I miss something? Yes, it doesn't exist. Is it because of it being temporary as per the following comment: "/* * XXX: This is the temporary way to update the plane registers until we get * around to using the usual plane update functions for MMIO flips */ " > -Daniel >
On 04/21/2015 05:55 AM, Jindal, Sonika wrote: > On 4/20/2015 11:14 PM, Daniel Vetter wrote: >> On Mon, Apr 20, 2015 at 05:38:20PM +0100, Tvrtko Ursulin wrote: >>> >>> On 04/20/2015 05:22 PM, Daniel Vetter wrote: >>>> On Mon, Apr 20, 2015 at 04:22:48PM +0100, Tvrtko Ursulin wrote: >>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>>> >>>>> Add Y tiling support to skl_do_mmio_flip. >>>>> >>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>>> Cc: Damien Lespiau <damien.lespiau@intel.com> >>>>> Cc: Sonika Jindal <sonika.jindal@intel.com> >>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>>>> --- >>>>> Looks like we completely missed this completely parallel instance >>>>> of plane >>>>> programming code when working on Y tiling support! >>>>> >>>>> Will rotation need supporting here as well? >>>> >>>> Oh dear this is a bit ugly. We definitely need either this for 4.1 >>>> or take >>>> out the Y-tiling support for 4.1. Rotation only made 4.2, so I think >>>> we're good, but we do indeed need to take rotation into account here >>>> too. >>> >>> What is ugly, this patch or the overall situation? >>> >>>> And it seems like we also need a pile of igt testcases to make sure >>>> Y-tiling, rotation (even 180 probably) works correctly with >>>> pageflips. Yay >>>> for kms_flip growing even more. >>> >>> I added Y to kms_flip_tiling back then - but both that one and kms_flip >>> don't do any pipe crc. So not much use in automated fashion. And >>> testdisplay >>> apparently does not hit this path. >>> >>> I did not have working crc hw back then, maybe that has changed, I'll >>> check >>> it and then, depending on the outcome, see if I can add something >>> automated. >>> >>> Either way, adding this fix sounds better than reverting everything. >> >> On 2nd thought skl is still tagged as preliminary, so we don't have any >> issues here at all. Queued for -next, thanks for the patch. >> >> While looking at skl_do_mmio_flip I noticed though that rotation support >> seems to be completely absent. Or did I miss something? > Yes, it doesn't exist. > Is it because of it being temporary as per the following comment: > "/* > * XXX: This is the temporary way to update the plane registers until > we get > * around to using the usual plane update functions for MMIO flips > */ > " Question is what are the prerequisites to start using plane update vfunc from here? Could we just do it now? Regards, Tvrtko
On Tue, Apr 21, 2015 at 10:05:46AM +0100, Tvrtko Ursulin wrote: > > On 04/21/2015 05:55 AM, Jindal, Sonika wrote: > >On 4/20/2015 11:14 PM, Daniel Vetter wrote: > >>On Mon, Apr 20, 2015 at 05:38:20PM +0100, Tvrtko Ursulin wrote: > >>> > >>>On 04/20/2015 05:22 PM, Daniel Vetter wrote: > >>>>On Mon, Apr 20, 2015 at 04:22:48PM +0100, Tvrtko Ursulin wrote: > >>>>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>>> > >>>>>Add Y tiling support to skl_do_mmio_flip. > >>>>> > >>>>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>>>Cc: Damien Lespiau <damien.lespiau@intel.com> > >>>>>Cc: Sonika Jindal <sonika.jindal@intel.com> > >>>>>Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > >>>>>--- > >>>>>Looks like we completely missed this completely parallel instance > >>>>>of plane > >>>>>programming code when working on Y tiling support! > >>>>> > >>>>>Will rotation need supporting here as well? > >>>> > >>>>Oh dear this is a bit ugly. We definitely need either this for 4.1 > >>>>or take > >>>>out the Y-tiling support for 4.1. Rotation only made 4.2, so I think > >>>>we're good, but we do indeed need to take rotation into account here > >>>>too. > >>> > >>>What is ugly, this patch or the overall situation? > >>> > >>>>And it seems like we also need a pile of igt testcases to make sure > >>>>Y-tiling, rotation (even 180 probably) works correctly with > >>>>pageflips. Yay > >>>>for kms_flip growing even more. > >>> > >>>I added Y to kms_flip_tiling back then - but both that one and kms_flip > >>>don't do any pipe crc. So not much use in automated fashion. And > >>>testdisplay > >>>apparently does not hit this path. > >>> > >>>I did not have working crc hw back then, maybe that has changed, I'll > >>>check > >>>it and then, depending on the outcome, see if I can add something > >>>automated. > >>> > >>>Either way, adding this fix sounds better than reverting everything. > >> > >>On 2nd thought skl is still tagged as preliminary, so we don't have any > >>issues here at all. Queued for -next, thanks for the patch. > >> > >>While looking at skl_do_mmio_flip I noticed though that rotation support > >>seems to be completely absent. Or did I miss something? > >Yes, it doesn't exist. > >Is it because of it being temporary as per the following comment: > >"/* > > * XXX: This is the temporary way to update the plane registers until > >we get > > * around to using the usual plane update functions for MMIO flips > > */ > >" > > Question is what are the prerequisites to start using plane update vfunc > from here? Could we just do it now? I'm not sure whether we can do some fancy intermediate step, in the end we'll need async atomic plane updates so that we can use the helpers to remap legacy page flips to atomic. But in any case if we don't fix this up right away we need to track this as a blocker to lift skl prelimary hw support. Adding Damien as an fyi. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bac82d5..4d37f74 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10480,23 +10480,34 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc) struct drm_device *dev = intel_crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct drm_framebuffer *fb = intel_crtc->base.primary->fb; - struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); - struct drm_i915_gem_object *obj = intel_fb->obj; const enum pipe pipe = intel_crtc->pipe; u32 ctl, stride; ctl = I915_READ(PLANE_CTL(pipe, 0)); ctl &= ~PLANE_CTL_TILED_MASK; - if (obj->tiling_mode == I915_TILING_X) + switch (fb->modifier[0]) { + case DRM_FORMAT_MOD_NONE: + break; + case I915_FORMAT_MOD_X_TILED: ctl |= PLANE_CTL_TILED_X; + break; + case I915_FORMAT_MOD_Y_TILED: + ctl |= PLANE_CTL_TILED_Y; + break; + case I915_FORMAT_MOD_Yf_TILED: + ctl |= PLANE_CTL_TILED_YF; + break; + default: + MISSING_CASE(fb->modifier[0]); + } /* * The stride is either expressed as a multiple of 64 bytes chunks for * linear buffers or in number of tiles for tiled buffers. */ - stride = fb->pitches[0] >> 6; - if (obj->tiling_mode == I915_TILING_X) - stride = fb->pitches[0] >> 9; /* X tiles are 512 bytes wide */ + stride = fb->pitches[0] / + intel_fb_stride_alignment(dev, fb->modifier[0], + fb->pixel_format); /* * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on