Re: Support for early wakeup in DRM
On 2020-02-21 09:20, Daniel Vetter wrote: On Thu, Feb 20, 2020 at 01:24:00PM -0800, jsa...@codeaurora.org wrote: On 2020-02-20 12:14, Daniel Vetter wrote: > On Thu, Feb 20, 2020 at 10:45:57AM -0800, jsa...@codeaurora.org wrote: > > Hello All, > > > > I am seeking recommendations for DRM compatible methods of updating > > the > HW > > other than frame commit path. When exiting idle/runtime_suspend, the > driver > > votes for a bunch of resources including power/clk/bandwidth as a part > of > > first commit handling. This usually adds a few millisecond delay > > before > > processing the frame. The requirement is to find possible ways to > > reduce > > this delay by providing an early intimation to the framework to > "prepare" > > the HW by voting for the resources and keep the HW ready to process an > > imminent frame commit. Especially in performance oriented Automotive > world, > > these delays are very time critical and we are working on ways to > mitigate > > them. > > > > > > > > DRM framework converges all the parameters affecting the HW in terms > > of > DRM > > properties in a single COMMIT call. To address the above issue, we > > need > a > > parallel channel which should allow the framework to make necessary > changes > > to the HW without violating the master access privileges. > > > > > > > > Before resorting to custom downstream ways, I want to check with the > > community for folks who might have encountered and resolved such > > issues. > > Just enable the display, which will grab all the clocks and everything? > Once the display is on a commit should be possible on the next frame, at > least for well-working drivers. > -Daniel > I believe even to turn on the display, DRM will need an explicit commit (probably without any planes/pixel buffers). For cases like smart panels, where we can keep the panel on(panel internal RAM refresh) and power collapse the display HW, resuming back with an explicit commit will push a black (or default color programmed in the HW) frame causing a glitch. Uh, you might want to look into the self-refresh helpers, which do this without black frames and stuff. I believe you are referring to Sean's PSR changes: https://patchwork.freedesktop.org/series/57366/ Will take a look. Thanks and Regards, Jeykumar S. But yeah if there's really a gap here (and not just you folks creatively abusing atomic kms in ways that it was not meant to be used) then we can add a property that forbids power optimization and guarantee that you can do the next screen update immediately. And then we can merge that with all the usual requirements (driver implementation that works, open source userspace, igt testcase, the full deal). But it still feels like you're trying to do something automatically that's not meant to work like this. Cheers, Daniel Thanks and Regards, Jeykumar S. > > > > > > > > Thanks and Regards, > > > > Jeykumar S > > > > Qualcomm Inc. > > > > > > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Support for early wakeup in DRM
On 2020-02-20 12:14, Daniel Vetter wrote: On Thu, Feb 20, 2020 at 10:45:57AM -0800, jsa...@codeaurora.org wrote: Hello All, I am seeking recommendations for DRM compatible methods of updating the HW other than frame commit path. When exiting idle/runtime_suspend, the driver votes for a bunch of resources including power/clk/bandwidth as a part of first commit handling. This usually adds a few millisecond delay before processing the frame. The requirement is to find possible ways to reduce this delay by providing an early intimation to the framework to "prepare" the HW by voting for the resources and keep the HW ready to process an imminent frame commit. Especially in performance oriented Automotive world, these delays are very time critical and we are working on ways to mitigate them. DRM framework converges all the parameters affecting the HW in terms of DRM properties in a single COMMIT call. To address the above issue, we need a parallel channel which should allow the framework to make necessary changes to the HW without violating the master access privileges. Before resorting to custom downstream ways, I want to check with the community for folks who might have encountered and resolved such issues. Just enable the display, which will grab all the clocks and everything? Once the display is on a commit should be possible on the next frame, at least for well-working drivers. -Daniel I believe even to turn on the display, DRM will need an explicit commit (probably without any planes/pixel buffers). For cases like smart panels, where we can keep the panel on(panel internal RAM refresh) and power collapse the display HW, resuming back with an explicit commit will push a black (or default color programmed in the HW) frame causing a glitch. Thanks and Regards, Jeykumar S. Thanks and Regards, Jeykumar S Qualcomm Inc. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Support for early wakeup in DRM
Hello All, I am seeking recommendations for DRM compatible methods of updating the HW other than frame commit path. When exiting idle/runtime_suspend, the driver votes for a bunch of resources including power/clk/bandwidth as a part of first commit handling. This usually adds a few millisecond delay before processing the frame. The requirement is to find possible ways to reduce this delay by providing an early intimation to the framework to "prepare" the HW by voting for the resources and keep the HW ready to process an imminent frame commit. Especially in performance oriented Automotive world, these delays are very time critical and we are working on ways to mitigate them. DRM framework converges all the parameters affecting the HW in terms of DRM properties in a single COMMIT call. To address the above issue, we need a parallel channel which should allow the framework to make necessary changes to the HW without violating the master access privileges. Before resorting to custom downstream ways, I want to check with the community for folks who might have encountered and resolved such issues. Thanks and Regards, Jeykumar S Qualcomm Inc. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC] checking drm_framebuffer against config width/height
Hello All, I bumped into the below check [1] enforced in drm_framebuffer creation which checks the requested framebuffer width/height parameters against the drm mode config width/height limits. As I understand, drm_mode_config: min/max width/height indicate the upper and lower bounds of the display panel (drm_connector) resolutions the DRM device can support. But the pixel processing pipeline can apply cropping/scaling transformations on much larger input framebuffers and flip the buffers within the display resolution. Such configurations are very common and the final resolution will be still within drm_mode_config bounds. So I believe the checks should be relaxed / removed from the drm_framebuffer creation api's. If my understanding is incorrect, could somehow explain the motivation behind having these checks here? Thanks and Regards, Jeykumar S. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv ers/gpu/drm/drm_framebuffer.c?h=v5.3#n303 struct drm_framebuffer * drm_internal_framebuffer_create(struct drm_device *dev, const struct drm_mode_fb_cmd2 *r, struct drm_file *file_priv) { /* snip */ if ((config->min_width > r->width) || (r->width > config->max_width)) { DRM_DEBUG_KMS("bad framebuffer width %d, should be >= %d && <= %d\n", r->width, config->min_width, config->max_width); return ERR_PTR(-EINVAL); } if ((config->min_height > r->height) || (r->height > config->max_height)) { DRM_DEBUG_KMS("bad framebuffer height %d, should be >= %d && <= %d\n", r->height, config->min_height, config->max_height); return ERR_PTR(-EINVAL); } /* snip */ } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [bug report] drm/msm: Add SDM845 DPU support
Thanks for reporting the issue Dan. Posted the patch below as the fix. https://patchwork.freedesktop.org/series/50637/ Thanks and Regards, Jeykumar S. -Original Message- From: Dan Carpenter Sent: Monday, October 1, 2018 2:39 AM To: jsa...@codeaurora.org Cc: dri-devel@lists.freedesktop.org Subject: [bug report] drm/msm: Add SDM845 DPU support Hello Jeykumar Sankaran, The patch 25fdd5933e4c: "drm/msm: Add SDM845 DPU support" from Jun 27, 2018, leads to the following static checker warning: drivers/gpu/drm/msm/msm_drv.c:562 msm_drm_init() warn: 'priv->disp_thread[i].thread' isn't an ERR_PTR drivers/gpu/drm/msm/msm_drv.c 540 /** 541 * this priority was found during empiric testing to have appropriate 542 * realtime scheduling to process display updates and interact with 543 * other real time and normal priority task 544 */ 545 param.sched_priority = 16; 546 for (i = 0; i < priv->num_crtcs; i++) { 547 548 /* initialize display thread */ 549 priv->disp_thread[i].crtc_id = priv->crtcs[i]->base.id; 550 kthread_init_worker(&priv->disp_thread[i].worker); 551 priv->disp_thread[i].dev = ddev; 552 priv->disp_thread[i].thread = ^^^ 553 kthread_run(kthread_worker_fn, 554 &priv->disp_thread[i].worker, 555 "crtc_commit:%d", priv->disp_thread[i].crtc_id); 556 ret = sched_setscheduler(priv->disp_thread[i].thread, ^^^ Only pass valid pointers to this because it's going to dereference it. 557 SCHED_FIFO, ¶m); 558 if (ret) 559 pr_warn("display thread priority update failed: %d\n", 560 ret); 561 562 if (IS_ERR(priv->disp_thread[i].thread)) { ^^^ Too late. 563 dev_err(dev, "failed to create crtc_commit kthread\n"); 564 priv->disp_thread[i].thread = NULL; 565 } 566 567 /* initialize event thread */ 568 priv->event_thread[i].crtc_id = priv->crtcs[i]->base.id; 569 kthread_init_worker(&priv->event_thread[i].worker); 570 priv->event_thread[i].dev = ddev; 571 priv->event_thread[i].thread = 572 kthread_run(kthread_worker_fn, 573 &priv->event_thread[i].worker, 574 "crtc_event:%d", priv->event_thread[i].crtc_id); regards, dan carpenter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH 04/11] drm/msm: Move implicit sync fence handling to prepare_fb
On 2018-02-28 11:18, Sean Paul wrote: This is another piece that can be moved out of atomic to facilitate using the atomic helpers. Change-Id: I6dc3c4e5df508942bbc378c73a44e46e511b8469 Signed-off-by: Sean Paul Reviewed-by: Jeykumar Sankaran --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 8 drivers/gpu/drm/msm/msm_atomic.c | 13 - 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 834dcc0bfefd..29e72b39fd72 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -2720,6 +2720,8 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane, struct dpu_plane_rot_state *new_rstate; struct dpu_hw_fmt_layout layout; struct msm_gem_address_space *aspace; + struct msm_gem_object *msm_obj; + struct dma_fence *fence; int ret; if (!new_state->fb) @@ -2761,6 +2763,12 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane, return ret; } + /* To support implicit sync, set a fence on the plane if appropriate */ + msm_obj = to_msm_bo(msm_framebuffer_bo(fb, 0)); + fence = reservation_object_get_excl_rcu(msm_obj->resv); + if (fence) + drm_atomic_set_fence_for_plane(new_state, fence); + return 0; } diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index eb2ccda5da0f..3a18bd3dc215 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -282,19 +282,6 @@ int msm_atomic_commit(struct drm_device *dev, for_each_new_crtc_in_state(state, crtc, crtc_state, i) c->crtc_mask |= drm_crtc_mask(crtc); - /* -* Figure out what fence to wait for: -*/ - for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { - if ((new_plane_state->fb != old_plane_state->fb) && new_plane_state->fb) { - struct drm_gem_object *obj = msm_framebuffer_bo(new_plane_state->fb, 0); - struct msm_gem_object *msm_obj = to_msm_bo(obj); - struct dma_fence *fence = reservation_object_get_excl_rcu(msm_obj->resv); - - drm_atomic_set_fence_for_plane(new_plane_state, fence); - } - } - /* * Wait for pending updates on any of the same crtc's and then * mark our set of crtc's as busy: ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH 01/11] drm/msm: Skip seamless disables in crtc/encoder
On 2018-02-28 11:18, Sean Paul wrote: Instead of duplicating whole swaths of atomic helper functions (which are already out-of-date), just skip the encoder/crtc disables in the .disable hooks. Change-Id: I7bd9183ae60624204fb1de9550656b776efc7202 Signed-off-by: Sean Paul Can you consider getting rid of these checks? --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 8 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 + drivers/gpu/drm/msm/msm_atomic.c| 185 +--- 3 files changed, 17 insertions(+), 184 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 3cdf1e3d9d96..a3ab6ed2bf1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -3393,6 +3393,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc) { struct dpu_crtc *dpu_crtc; struct dpu_crtc_state *cstate; + struct drm_display_mode *mode; struct drm_encoder *encoder; struct msm_drm_private *priv; unsigned long flags; @@ -3407,8 +3408,15 @@ static void dpu_crtc_disable(struct drm_crtc *crtc) } dpu_crtc = to_dpu_crtc(crtc); cstate = to_dpu_crtc_state(crtc->state); + mode = &cstate->base.adjusted_mode; priv = crtc->dev->dev_private; + if (msm_is_mode_seamless(mode) || msm_is_mode_seamless_vrr(mode) || + msm_is_mode_seamless_dms(mode)) { + DPU_DEBUG("Seamless mode is being applied, skip disable\n"); + return; + } + Another topic of discussion which should be brought up with dri-devel. May not be common in PC world, but there are a handful of mobile OEM's using panels which supports more than one resolution. Primary use cases involve "seamless" switching to optimized display resolution when streaming content changes resolutions or rendering lossless data. Jeykumar S. DPU_DEBUG("crtc%d\n", crtc->base.id); if (dpu_kms_is_suspend_state(crtc->dev)) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 3d168fa09f3f..28ceb589ee40 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2183,6 +2183,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc) struct dpu_encoder_virt *dpu_enc = NULL; struct msm_drm_private *priv; struct dpu_kms *dpu_kms; + struct drm_display_mode *mode; int i = 0; if (!drm_enc) { @@ -2196,6 +2197,13 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc) return; } + mode = &drm_enc->crtc->state->adjusted_mode; + if (msm_is_mode_seamless(mode) || msm_is_mode_seamless_vrr(mode) || + msm_is_mode_seamless_dms(mode)) { + DPU_DEBUG("Seamless mode is being applied, skip disable\n"); + return; + } + dpu_enc = to_dpu_encoder_virt(drm_enc); DPU_DEBUG_ENC(dpu_enc, "\n"); diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 46536edb72ee..5cfb80345052 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -84,189 +84,6 @@ static void msm_atomic_wait_for_commit_done( } } -static void -msm_disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) -{ - struct drm_connector *connector; - struct drm_connector_state *old_conn_state, *new_conn_state; - struct drm_crtc *crtc; - struct drm_crtc_state *old_crtc_state, *new_crtc_state; - int i; - - for_each_oldnew_connector_in_state(old_state, connector, old_conn_state, new_conn_state, i) { - const struct drm_encoder_helper_funcs *funcs; - struct drm_encoder *encoder; - struct drm_crtc_state *old_crtc_state; - unsigned int crtc_idx; - - /* -* Shut down everything that's in the changeset and currently -* still on. So need to check the old, saved state. -*/ - if (!old_conn_state->crtc) - continue; - - crtc_idx = drm_crtc_index(old_conn_state->crtc); - old_crtc_state = drm_atomic_get_old_crtc_state(old_state, - old_conn_state->crtc); - - if (!old_crtc_state->active || - !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state)) - continue; - - encoder = old_conn_state->best_encoder; - - /* We shouldn't get this far if we didn't previously have -* an encoder.. but WARN_ON() rather than explode. -*/ - if (WARN_ON(!encoder)) - continue; - - if (msm_is_mode_seamless( - &connector->encoder->crtc->state->mode) || - msm_is_mode_seamless_
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On 2018-03-01 07:27, Sean Paul wrote: On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsa...@codeaurora.org wrote: On 2018-02-28 11:19, Sean Paul wrote: > Moving further towards switching fully to the the atomic helpers, this > patch removes the hand-rolled kthread nonblock commit code and uses the > atomic helpers commit_work model. > > There's still a lot of copypasta here, but it's still needed to > facilitate the swap_state and prepare_fence private functions. These > will be sorted out in a follow-on patch. > > Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 > Signed-off-by: Sean Paul > --- > drivers/gpu/drm/msm/msm_atomic.c | 199 ++- > drivers/gpu/drm/msm/msm_drv.c| 1 - > drivers/gpu/drm/msm/msm_drv.h| 4 - > 3 files changed, 35 insertions(+), 169 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_atomic.c > b/drivers/gpu/drm/msm/msm_atomic.c > index 3a18bd3dc215..7e54eb65d096 100644 > --- a/drivers/gpu/drm/msm/msm_atomic.c > +++ b/drivers/gpu/drm/msm/msm_atomic.c > @@ -21,51 +21,6 @@ > #include "msm_gem.h" > #include "msm_fence.h" > > -struct msm_commit { > - struct drm_device *dev; > - struct drm_atomic_state *state; > - uint32_t crtc_mask; > - bool nonblock; > - struct kthread_work commit_work; > -}; > - > -/* block until specified crtcs are no longer pending update, and > - * atomically mark them as pending update > - */ > -static int start_atomic(struct msm_drm_private *priv, uint32_t > crtc_mask) > -{ > - int ret; > - > - spin_lock(&priv->pending_crtcs_event.lock); > - ret = wait_event_interruptible_locked(priv->pending_crtcs_event, > - !(priv->pending_crtcs & crtc_mask)); > - if (ret == 0) { > - DBG("start: %08x", crtc_mask); > - priv->pending_crtcs |= crtc_mask; > - } > - spin_unlock(&priv->pending_crtcs_event.lock); > - > - return ret; > -} > - > -/* clear specified crtcs (no longer pending update) > - */ > -static void end_atomic(struct msm_drm_private *priv, uint32_t > crtc_mask) > -{ > - spin_lock(&priv->pending_crtcs_event.lock); > - DBG("end: %08x", crtc_mask); > - priv->pending_crtcs &= ~crtc_mask; > - wake_up_all_locked(&priv->pending_crtcs_event); > - spin_unlock(&priv->pending_crtcs_event.lock); > -} > - > -static void commit_destroy(struct msm_commit *c) > -{ > - end_atomic(c->dev->dev_private, c->crtc_mask); > - if (c->nonblock) > - kfree(c); > -} > - > static void msm_atomic_wait_for_commit_done( >struct drm_device *dev, >struct drm_atomic_state *old_state) > @@ -118,6 +73,10 @@ static void msm_atomic_commit_tail(struct > drm_atomic_state *state) > >msm_atomic_wait_for_commit_done(dev, state); > > + drm_atomic_helper_commit_hw_done(state); > + > + drm_atomic_helper_wait_for_vblanks(dev, state); > + >drm_atomic_helper_cleanup_planes(dev, state); > >kms->funcs->complete_commit(kms, state); > @@ -126,109 +85,25 @@ static void msm_atomic_commit_tail(struct > drm_atomic_state *state) > /* The (potentially) asynchronous part of the commit. At this point > * nothing can fail short of armageddon. > */ > -static void complete_commit(struct msm_commit *c) > +static void commit_tail(struct drm_atomic_state *state) > { > - struct drm_atomic_state *state = c->state; > - struct drm_device *dev = state->dev; > + drm_atomic_helper_wait_for_fences(state->dev, state, false); > > - drm_atomic_helper_wait_for_fences(dev, state, false); > + drm_atomic_helper_wait_for_dependencies(state); > >msm_atomic_commit_tail(state); > > - drm_atomic_state_put(state); > -} > - > -static void _msm_drm_commit_work_cb(struct kthread_work *work) > -{ > - struct msm_commit *commit = NULL; > - > - if (!work) { > - DRM_ERROR("%s: Invalid commit work data!\n", __func__); > - return; > - } > - > - commit = container_of(work, struct msm_commit, commit_work); > - > - complete_commit(commit); > - > - commit_destroy(commit); > -} > - > -static struct msm_commit *commit_init(struct drm_atomic_state *state, > - bool nonblock) > -{ > - struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL); > + drm_atomic_helper_commit_cleanup_done(state); > > - if (!c) > - return NULL; > - > - c->dev = state->dev; > - c->state = state; > - c->nonblock = nonblock; > - > - kthread_init_work(&c->commit_work, _msm_drm_commit_work_cb); > - > - return c; > + drm_atomic_state_put(state); > } > > -/* Start display thread function */ > -static void msm_atomic_commit_dispatch(struct drm_device *dev, > - struct drm_atomic_state *state, struct msm_commit *commit) > +static void commit_work(struct work_struct *work) > { > - struct msm_drm_private *priv = dev->dev_private; > - struct drm_crtc *crtc = NULL; > - struct drm_crtc_state *new_crtc_state = NULL; > - int ret = -EINVAL, i = 0, j = 0; > - bool nonblock; > - > - /* cache since work will kfree commit in non-blocking case */ > - nonblock = commit->nonblock; > - > -
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On 2018-02-28 11:19, Sean Paul wrote: Moving further towards switching fully to the the atomic helpers, this patch removes the hand-rolled kthread nonblock commit code and uses the atomic helpers commit_work model. There's still a lot of copypasta here, but it's still needed to facilitate the swap_state and prepare_fence private functions. These will be sorted out in a follow-on patch. Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_atomic.c | 199 ++- drivers/gpu/drm/msm/msm_drv.c| 1 - drivers/gpu/drm/msm/msm_drv.h| 4 - 3 files changed, 35 insertions(+), 169 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 3a18bd3dc215..7e54eb65d096 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -21,51 +21,6 @@ #include "msm_gem.h" #include "msm_fence.h" -struct msm_commit { - struct drm_device *dev; - struct drm_atomic_state *state; - uint32_t crtc_mask; - bool nonblock; - struct kthread_work commit_work; -}; - -/* block until specified crtcs are no longer pending update, and - * atomically mark them as pending update - */ -static int start_atomic(struct msm_drm_private *priv, uint32_t crtc_mask) -{ - int ret; - - spin_lock(&priv->pending_crtcs_event.lock); - ret = wait_event_interruptible_locked(priv->pending_crtcs_event, - !(priv->pending_crtcs & crtc_mask)); - if (ret == 0) { - DBG("start: %08x", crtc_mask); - priv->pending_crtcs |= crtc_mask; - } - spin_unlock(&priv->pending_crtcs_event.lock); - - return ret; -} - -/* clear specified crtcs (no longer pending update) - */ -static void end_atomic(struct msm_drm_private *priv, uint32_t crtc_mask) -{ - spin_lock(&priv->pending_crtcs_event.lock); - DBG("end: %08x", crtc_mask); - priv->pending_crtcs &= ~crtc_mask; - wake_up_all_locked(&priv->pending_crtcs_event); - spin_unlock(&priv->pending_crtcs_event.lock); -} - -static void commit_destroy(struct msm_commit *c) -{ - end_atomic(c->dev->dev_private, c->crtc_mask); - if (c->nonblock) - kfree(c); -} - static void msm_atomic_wait_for_commit_done( struct drm_device *dev, struct drm_atomic_state *old_state) @@ -118,6 +73,10 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state) msm_atomic_wait_for_commit_done(dev, state); + drm_atomic_helper_commit_hw_done(state); + + drm_atomic_helper_wait_for_vblanks(dev, state); + drm_atomic_helper_cleanup_planes(dev, state); kms->funcs->complete_commit(kms, state); @@ -126,109 +85,25 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state) /* The (potentially) asynchronous part of the commit. At this point * nothing can fail short of armageddon. */ -static void complete_commit(struct msm_commit *c) +static void commit_tail(struct drm_atomic_state *state) { - struct drm_atomic_state *state = c->state; - struct drm_device *dev = state->dev; + drm_atomic_helper_wait_for_fences(state->dev, state, false); - drm_atomic_helper_wait_for_fences(dev, state, false); + drm_atomic_helper_wait_for_dependencies(state); msm_atomic_commit_tail(state); - drm_atomic_state_put(state); -} - -static void _msm_drm_commit_work_cb(struct kthread_work *work) -{ - struct msm_commit *commit = NULL; - - if (!work) { - DRM_ERROR("%s: Invalid commit work data!\n", __func__); - return; - } - - commit = container_of(work, struct msm_commit, commit_work); - - complete_commit(commit); - - commit_destroy(commit); -} - -static struct msm_commit *commit_init(struct drm_atomic_state *state, - bool nonblock) -{ - struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL); + drm_atomic_helper_commit_cleanup_done(state); - if (!c) - return NULL; - - c->dev = state->dev; - c->state = state; - c->nonblock = nonblock; - - kthread_init_work(&c->commit_work, _msm_drm_commit_work_cb); - - return c; + drm_atomic_state_put(state); } -/* Start display thread function */ -static void msm_atomic_commit_dispatch(struct drm_device *dev, - struct drm_atomic_state *state, struct msm_commit *commit) +static void commit_work(struct work_struct *work) { - struct msm_drm_private *priv = dev->dev_private; - struct drm_crtc *crtc = NULL; - struct drm_crtc_state *new_crtc_state = NULL; - int ret = -EINVAL, i = 0, j = 0; - bool nonblock; - - /* cache since work will kfree commit in non-blocking case */ - nonblock = commit->nonblock; - - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { - for (j =
Re: [RFC PULL] Add Display Support for Qualcomm SDM845
On 2/13/2018 12:00 PM, Rob Clark wrote: On Tue, Feb 13, 2018 at 2:18 PM, Sean Paul wrote: Hi dri-devel, Qualcomm has been working for the past few weeks on forward porting their downstream drm driver from 4.14 to mainline. Please consider this PR as a request for review, rather than an attempt at mainlining the code as it currently stands. The goal is get this driver in shape over the next coming months. In the meantime, I'll be hosting a tree here [1] to stage the fixes. Patches will be posted and reviewed on linux-arm-...@vger.kernel.org. Once things look good, I'll send another pull 4realz. To get the ball rolling, I've done some review on the new connector code, my comments are below. Thanks in advance for your constructive feedback :) Sean [1]- git://people.freedesktop.org/~seanpaul/dpu-staging Review feedback: just so others aren't confused, s/sde/dpu/g in the list below (we did our DAL->DC rename before sending first RFC :-P) - Solve the splash screen handling (or remove it) - Simplify devicetree binding (remove register offsets) feedback from reviewing sde_connector.c: - Rationalize backlight implementation in sde_connector (display_count static) - Sort out the dsi event passing between dsi/encoder/connector (move to encoder) - include/uapi/drm/msm_drm_pp.h needs opensource userspace (or removal) - connector->state access violations reading/writing mode_info - s/sde_rect/drm_rect/ - sde_kms_info usage needs to be replaced with formal data structures (not stringified keypairs) - sde_connector_ops needs to be trimmed, duplicates connector helpers, info hooks circumvent state, and other hooks should be stored in state or prepopulated (get_dst_format) - sde_connector_get_dpms unused - esd status check should migrate to encoder from connector - backlight should be handled in panel drivers, not in the generic connector/dsi encoder - sde_connector_helper_bridge_disable is called from encoder and calls back into set_power encoder function. if backlight, and esd status are removed, pre_kickoff can probably go away - sde_connector_clk_ctrl is another example of encoder->connector->encoder call - RETIRE_FENCE connector property should be removed, opting for the native atomic fences - ROI (regions of interest) should be expressed per-plane instead of connector. there is work ongoing to support dirty_rects per-plane by Deepak Singh Rawat and Lukasz Spintzyk - Uma Shankar has proposed HDR source metadata properties on the list, we should pivot to those instead of hand-rolling them in the sde driver - Convert HDCP implementation to upstream Content Protection property - Merge dsi and dsi_staging into one driver - Writeback connector has been proposed by ARM (Liviu Dudau and Brian Starkey), we should work with their proposal instead of rolling OUT_FB ourselves - sde_connector_set_property should be replaced with atomic helper - dsi hotplug can probably be punted to the panel driver - dpms should switch to enable/disable (or at least use the atomic helpers) - dsi mode handling should also defer to the panel driver - SDE_WB_CONFIG ioctl should be removed in favor of the existing ioctl to add user-defined modes - dp implementation should use the existing dp helpers wherever possible - lots of duplicated structures in dsi_defs.h that can be replaced with existing drm structs - mode_valid should be split up and implemented directly in connector/encoder as appropriate - sde_connector->aspace seems like it's unused? To add to that, a few things I've noticed (but I've mostly only gotten as far as the front-end of the pipeline, ie. dpu_plane): - It looks like, as was the case with mdp4/mdp5 (and really most other hw) there are still unequal capabilities for planes (ie. some can do YUV, scaling, etc). But drm-hwc (or weston) isn't going to know about that, so I think we'll need to add support for dynamically assigning dpu_plane::pipe, similar to what mdp5 does w/ plane<->hwpipe. (Which I actually added specifically for drm-hwc ;-)) We are working on plane virtualization focused primarily to support 4K / wider displays which cannot be catered by one hwpipe. The plan is to gang up two *fixed* hwpipes of similar capabilities (in terms of formats, sub hw blocks like scalar, post processing ) and expose them as a single plane so that user space compositor ( drm-hwc or weston) need not worry about max pixel width supported by the planes. But mapping planes <-> hwpipe dynamically may need more than just virtualization. Kernel need to keep track of hwpipes especially when dealing with multiple displays. And it get complicated when planes start moving around CRTC's for different sized buffers. Given that DRM exposes planes with its own list of format/modifiers, I think its not that big of a deal for a compositor to look up for plane to support the format and use it. - I *think* we also need the same trick for