from:"jsanka"

Re: Support for early wakeup in DRM

2020-02-22 Thread jsanka


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

2020-02-20 Thread jsanka


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

2020-02-20 Thread jsanka

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

2019-09-18 Thread jsanka

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

2018-10-05 Thread jsanka

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(>disp_thread[i].worker);
   551  priv->disp_thread[i].dev = ddev;
   552  priv->disp_thread[i].thread =
^^^

   553  kthread_run(kthread_worker_fn,
   554  >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,
);
   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(>event_thread[i].worker);
   570  priv->event_thread[i].dev = ddev;
   571  priv->event_thread[i].thread =
   572  kthread_run(kthread_worker_fn,
   573  >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

2018-03-02 Thread jsanka


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

2018-03-02 Thread jsanka


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 = >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 = _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(
-   >encoder->crtc->state->mode) ||
-

Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit

2018-03-01 Thread jsanka


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(>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(>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(>pending_crtcs_event.lock);
> -  DBG("end: %08x", crtc_mask);
> -  priv->pending_crtcs &= ~crtc_mask;
> -  wake_up_all_locked(>pending_crtcs_event);
> -  spin_unlock(>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(>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

2018-02-28 Thread jsanka


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(>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(>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(>pending_crtcs_event.lock);
-   DBG("end: %08x", crtc_mask);
-   priv->pending_crtcs &= ~crtc_mask;
-   wake_up_all_locked(>pending_crtcs_event);
-   spin_unlock(>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(>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 = 0; j <

Re: [RFC PULL] Add Display Support for Qualcomm SDM845

2018-02-14 Thread jsanka




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

Re: Support for early wakeup in DRM

Re: Support for early wakeup in DRM

Support for early wakeup in DRM

[RFC] checking drm_framebuffer against config width/height

RE: [bug report] drm/msm: Add SDM845 DPU support

Re: [DPU PATCH 04/11] drm/msm: Move implicit sync fence handling to prepare_fb

Re: [DPU PATCH 01/11] drm/msm: Skip seamless disables in crtc/encoder

Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit

Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit

Re: [RFC PULL] Add Display Support for Qualcomm SDM845

10 matches

Site Navigation

Mail list logo

Footer information