| Message ID | 1416276639-30297-6-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=290/291->291/291
PNV: pass/total=356/356->356/356
ILK: pass/total=371/372->371/372
IVB: pass/total=544/546->545/546
SNB: pass/total=424/425->424/425
HSW: pass/total=595/595->595/595
BDW: pass/total=432/435->433/435
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
BYT: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M36)PASS(9, M31M36) -> TIMEOUT(3, M31)PASS(1, M31)
ILK: Intel_gpu_tools, igt_kms_flip_bcs-wf_vblank-vs-dpms, PASS(7, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M37)PASS(9, M26M37) -> FAIL(3, M26)PASS(1, M26)
IVB: Intel_gpu_tools, igt_kms_3d, DMESG_WARN(3, M4)PASS(1, M21) -> DMESG_WARN(1, M4)PASS(3, M4)
IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-random, DMESG_WARN(1, M21)PASS(9, M21M34M4) -> PASS(4, M4)
IVB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M21)PASS(9, M21M34M4) -> TIMEOUT(3, M4)PASS(1, M4)
SNB: Intel_gpu_tools, igt_kms_3d, DMESG_WARN(3, M22)PASS(1, M35) -> DMESG_WARN(1, M22)PASS(3, M22)
SNB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M35)PASS(9, M35M22) -> TIMEOUT(3, M22)PASS(1, M22)
BDW: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(1, M28)PASS(9, M30M28) -> TIMEOUT(3, M30)PASS(1, M30)
On Mon, Nov 17, 2014 at 06:10:39PM -0800, Matt Roper wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > After some refactor intel_primary_plane_setplane() does the same > as intel_pipe_set_base() so we can get rid of it and replace the calls > with intel_primary_plane_setplane(). > > v2: take Ville's comments: > - get the right arguments for update_plane() > - use drm_crtc_get_hv_timing() > > v3 (by Matt): > - Rebase to latest di-nightly codebase > - Use primary->funcs->update_plane() in __intel_set_mode() > - Use primary->funcs->disable_plane() in intel_crtc_disable() > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 124 ++++++++--------------------------- > 1 file changed, 27 insertions(+), 97 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 1c1886e..968ef0b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2890,71 +2890,6 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) > crtc->config.pipe_src_h = adjusted_mode->crtc_vdisplay; > } > > -static int > -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > - struct drm_framebuffer *fb) > -{ > - struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - enum pipe pipe = intel_crtc->pipe; > - struct drm_framebuffer *old_fb = crtc->primary->fb; > - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb); > - int ret; > - > - if (intel_crtc_has_pending_flip(crtc)) { > - DRM_ERROR("pipe is still busy with an old pageflip\n"); > - return -EBUSY; > - } > - > - /* no fb bound */ > - if (!fb) { > - DRM_ERROR("No FB bound\n"); > - return 0; > - } > - > - if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) { > - DRM_ERROR("no plane for crtc: plane %c, num_pipes %d\n", > - plane_name(intel_crtc->plane), > - INTEL_INFO(dev)->num_pipes); > - return -EINVAL; > - } > - > - mutex_lock(&dev->struct_mutex); > - ret = intel_pin_and_fence_fb_obj(crtc->primary, fb, NULL); > - if (ret == 0) > - i915_gem_track_fb(old_obj, intel_fb_obj(fb), > - INTEL_FRONTBUFFER_PRIMARY(pipe)); > - mutex_unlock(&dev->struct_mutex); > - if (ret != 0) { > - DRM_ERROR("pin & fence failed\n"); > - return ret; > - } > - > - dev_priv->display.update_primary_plane(crtc, fb, x, y); > - > - if (intel_crtc->active) > - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe)); > - > - crtc->primary->fb = fb; > - crtc->x = x; > - crtc->y = y; > - > - if (old_fb) { > - if (intel_crtc->active && old_fb != fb) > - intel_wait_for_vblank(dev, intel_crtc->pipe); > - mutex_lock(&dev->struct_mutex); > - intel_unpin_fb_obj(old_obj); > - mutex_unlock(&dev->struct_mutex); > - } > - > - mutex_lock(&dev->struct_mutex); > - intel_update_fbc(dev); > - mutex_unlock(&dev->struct_mutex); > - > - return 0; > -} > - > static void intel_fdi_normal_train(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > @@ -5267,8 +5202,6 @@ static void intel_crtc_disable(struct drm_crtc *crtc) > struct drm_device *dev = crtc->dev; > struct drm_connector *connector; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct drm_i915_gem_object *old_obj = intel_fb_obj(crtc->primary->fb); > - enum pipe pipe = to_intel_crtc(crtc)->pipe; > > /* crtc should still be enabled when we disable it. */ > WARN_ON(!crtc->enabled); > @@ -5277,14 +5210,7 @@ static void intel_crtc_disable(struct drm_crtc *crtc) > intel_crtc_update_sarea(crtc, false); > dev_priv->display.off(crtc); > > - if (crtc->primary->fb) { > - mutex_lock(&dev->struct_mutex); > - intel_unpin_fb_obj(old_obj); > - i915_gem_track_fb(old_obj, NULL, > - INTEL_FRONTBUFFER_PRIMARY(pipe)); > - mutex_unlock(&dev->struct_mutex); > - crtc->primary->fb = NULL; > - } > + crtc->primary->funcs->disable_plane(crtc->primary); > > /* Update computed state. */ > list_for_each_entry(connector, &dev->mode_config.connector_list, head) { > @@ -9578,6 +9504,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > struct drm_framebuffer *old_fb = crtc->primary->fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_plane *primary = crtc->primary; > + struct intel_plane *intel_plane = to_intel_plane(primary); > enum pipe pipe = intel_crtc->pipe; > struct intel_unpin_work *work; > struct intel_engine_cs *ring; > @@ -9737,7 +9665,15 @@ free_work: > if (ret == -EIO) { > out_hang: > intel_crtc_wait_for_pending_flips(crtc); This should alread get called from the .update_plane() hook. > - ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb); > + ret = primary->funcs->update_plane(primary, crtc, fb, > + intel_plane->crtc_x, > + intel_plane->crtc_y, > + intel_plane->crtc_h, > + intel_plane->crtc_w, > + intel_plane->src_x, > + intel_plane->src_y, > + intel_plane->src_h, > + intel_plane->src_w); > if (ret == 0 && event) { > spin_lock_irq(&dev->event_lock); > drm_send_vblank_event(dev, pipe, event); > @@ -10913,26 +10849,15 @@ static int __intel_set_mode(struct drm_crtc *crtc, > * on the DPLL. > */ > for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) { > - struct drm_framebuffer *old_fb = crtc->primary->fb; > - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb); > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - > - mutex_lock(&dev->struct_mutex); > - ret = intel_pin_and_fence_fb_obj(crtc->primary, fb, NULL); > - if (ret != 0) { > - DRM_ERROR("pin & fence failed\n"); > - mutex_unlock(&dev->struct_mutex); > - goto done; > - } > - if (old_fb) > - intel_unpin_fb_obj(old_obj); > - i915_gem_track_fb(old_obj, obj, > - INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe)); > - mutex_unlock(&dev->struct_mutex); > + struct drm_plane *primary = intel_crtc->base.primary; > + int vdisplay, hdisplay; > > - crtc->primary->fb = fb; > - crtc->x = x; > - crtc->y = y; > + drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay); > + ret = primary->funcs->update_plane(primary, &intel_crtc->base, > + fb, 0, 0, > + hdisplay, vdisplay, > + x << 16, y << 16, > + hdisplay << 16, vdisplay << 16); > } > > /* Now enable the clocks, plane, pipe, and connectors that we set up. */ > @@ -11398,11 +11323,16 @@ static int intel_crtc_set_config(struct drm_mode_set *set) > disable_pipes); > } else if (config->fb_changed) { > struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc); > + struct drm_plane *primary = set->crtc->primary; > + int vdisplay, hdisplay; > > intel_crtc_wait_for_pending_flips(set->crtc); ditto Otherwise this looks nice. So assuming it works and we didn't miss anything important, and we can just sort out the remaining stereo doubling issue, I think this should fix some of the disappearing primary plane issues we have currently. I'm not quite sure if we're now close enough to unify all plane handling in intel_crtc_{enable,disable}_planes(). The trick with those is that they shoudln't affect the user visible state of the plane, so we need to first make sure that's kept separate. So I'm thinking we would want to virtualize the check/prepare/commit hooks and make sure all the user visible state is pulled out from them into the .update_plane/.disable_plane hooks. Does that sound like what other people had in mind? There's also the primary plane handling in the sprite code, but cleaning that up that may require a bit more work since that's actually an internal special case of a nuclear flip. > > - ret = intel_pipe_set_base(set->crtc, > - set->x, set->y, set->fb); > + drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay); > + ret = primary->funcs->update_plane(primary, set->crtc, set->fb, > + 0, 0, hdisplay, vdisplay, > + set->x << 16, set->y << 16, > + hdisplay << 16, vdisplay << 16); > > /* > * We need to make sure the primary plane is re-enabled if it > -- > 1.8.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 17 Nov 2014 18:10:39 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > -static int > -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > - struct drm_framebuffer *fb) > -{ I'm gonna miss this old function... But on the plus side I won't confuse pipe_set_base and set_pipe_base anymore, always got that wrong when searching.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1c1886e..968ef0b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2890,71 +2890,6 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) crtc->config.pipe_src_h = adjusted_mode->crtc_vdisplay; } -static int -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, - struct drm_framebuffer *fb) -{ - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; - struct drm_framebuffer *old_fb = crtc->primary->fb; - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb); - int ret; - - if (intel_crtc_has_pending_flip(crtc)) { - DRM_ERROR("pipe is still busy with an old pageflip\n"); - return -EBUSY; - } - - /* no fb bound */ - if (!fb) { - DRM_ERROR("No FB bound\n"); - return 0; - } - - if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) { - DRM_ERROR("no plane for crtc: plane %c, num_pipes %d\n", - plane_name(intel_crtc->plane), - INTEL_INFO(dev)->num_pipes); - return -EINVAL; - } - - mutex_lock(&dev->struct_mutex); - ret = intel_pin_and_fence_fb_obj(crtc->primary, fb, NULL); - if (ret == 0) - i915_gem_track_fb(old_obj, intel_fb_obj(fb), - INTEL_FRONTBUFFER_PRIMARY(pipe)); - mutex_unlock(&dev->struct_mutex); - if (ret != 0) { - DRM_ERROR("pin & fence failed\n"); - return ret; - } - - dev_priv->display.update_primary_plane(crtc, fb, x, y); - - if (intel_crtc->active) - intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe)); - - crtc->primary->fb = fb; - crtc->x = x; - crtc->y = y; - - if (old_fb) { - if (intel_crtc->active && old_fb != fb) - intel_wait_for_vblank(dev, intel_crtc->pipe); - mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(old_obj); - mutex_unlock(&dev->struct_mutex); - } - - mutex_lock(&dev->struct_mutex); - intel_update_fbc(dev); - mutex_unlock(&dev->struct_mutex); - - return 0; -} - static void intel_fdi_normal_train(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -5267,8 +5202,6 @@ static void intel_crtc_disable(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; struct drm_connector *connector; struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_i915_gem_object *old_obj = intel_fb_obj(crtc->primary->fb); - enum pipe pipe = to_intel_crtc(crtc)->pipe; /* crtc should still be enabled when we disable it. */ WARN_ON(!crtc->enabled); @@ -5277,14 +5210,7 @@ static void intel_crtc_disable(struct drm_crtc *crtc) intel_crtc_update_sarea(crtc, false); dev_priv->display.off(crtc); - if (crtc->primary->fb) { - mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(old_obj); - i915_gem_track_fb(old_obj, NULL, - INTEL_FRONTBUFFER_PRIMARY(pipe)); - mutex_unlock(&dev->struct_mutex); - crtc->primary->fb = NULL; - } + crtc->primary->funcs->disable_plane(crtc->primary); /* Update computed state. */ list_for_each_entry(connector, &dev->mode_config.connector_list, head) { @@ -9578,6 +9504,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *old_fb = crtc->primary->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_plane *primary = crtc->primary; + struct intel_plane *intel_plane = to_intel_plane(primary); enum pipe pipe = intel_crtc->pipe; struct intel_unpin_work *work; struct intel_engine_cs *ring; @@ -9737,7 +9665,15 @@ free_work: if (ret == -EIO) { out_hang: intel_crtc_wait_for_pending_flips(crtc); - ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb); + ret = primary->funcs->update_plane(primary, crtc, fb, + intel_plane->crtc_x, + intel_plane->crtc_y, + intel_plane->crtc_h, + intel_plane->crtc_w, + intel_plane->src_x, + intel_plane->src_y, + intel_plane->src_h, + intel_plane->src_w); if (ret == 0 && event) { spin_lock_irq(&dev->event_lock); drm_send_vblank_event(dev, pipe, event); @@ -10913,26 +10849,15 @@ static int __intel_set_mode(struct drm_crtc *crtc, * on the DPLL. */ for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) { - struct drm_framebuffer *old_fb = crtc->primary->fb; - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb); - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - - mutex_lock(&dev->struct_mutex); - ret = intel_pin_and_fence_fb_obj(crtc->primary, fb, NULL); - if (ret != 0) { - DRM_ERROR("pin & fence failed\n"); - mutex_unlock(&dev->struct_mutex); - goto done; - } - if (old_fb) - intel_unpin_fb_obj(old_obj); - i915_gem_track_fb(old_obj, obj, - INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe)); - mutex_unlock(&dev->struct_mutex); + struct drm_plane *primary = intel_crtc->base.primary; + int vdisplay, hdisplay; - crtc->primary->fb = fb; - crtc->x = x; - crtc->y = y; + drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay); + ret = primary->funcs->update_plane(primary, &intel_crtc->base, + fb, 0, 0, + hdisplay, vdisplay, + x << 16, y << 16, + hdisplay << 16, vdisplay << 16); } /* Now enable the clocks, plane, pipe, and connectors that we set up. */ @@ -11398,11 +11323,16 @@ static int intel_crtc_set_config(struct drm_mode_set *set) disable_pipes); } else if (config->fb_changed) { struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc); + struct drm_plane *primary = set->crtc->primary; + int vdisplay, hdisplay; intel_crtc_wait_for_pending_flips(set->crtc); - ret = intel_pipe_set_base(set->crtc, - set->x, set->y, set->fb); + drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay); + ret = primary->funcs->update_plane(primary, set->crtc, set->fb, + 0, 0, hdisplay, vdisplay, + set->x << 16, set->y << 16, + hdisplay << 16, vdisplay << 16); /* * We need to make sure the primary plane is re-enabled if it