All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Clark <robdclark@gmail.com>
To: Caleb Connolly <caleb.connolly@linaro.org>,
	Jordan Crouse <jordan@cosmicpenguin.net>
Cc: Dave Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	freedreno <freedreno@lists.freedesktop.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Abhinav Kumar <abhinavk@codeaurora.org>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Subject: Re: [early pull] drm/msm: drm-msm-next-2021-07-28 for v5.15
Date: Wed, 28 Jul 2021 18:02:28 -0700	[thread overview]
Message-ID: <CAF6AEGuJjH94s0ymARtEMUo2tBuaacx7e0nqOj7_j2SQQcUa9Q@mail.gmail.com> (raw)
In-Reply-To: <7553f3cd-61c8-3ece-14ec-6c0cf4ae0296@linaro.org>

Jordan, any idea if more frequent frequency changes would for some
reason make a630 grumpy?  I was expecting it should be somewhat
similar to a618 (same GMU fw, etc).  The main result of that patch
should be clamping to min freq when gpu goes idle, and the toggling
back to devfreq provided freq on idle->active transition.  So there
might be more frequent freq transitions.

Caleb, I don't suppose you could somehow delay starting UI and get
some traces?  Something along the lines of:

  localhost ~ # cd /sys/kernel/debug/tracing/
  localhost /sys/kernel/debug/tracing # echo 1 > events/drm_msm_gpu/enable
  localhost /sys/kernel/debug/tracing # echo 1 > tracing_on
  localhost /sys/kernel/debug/tracing # cat trace_pipe

Does adding an 'if (1) return' at the top of msm_devfreq_idle() help?
That should bypass the clamping to min freq when the GPU isn't doing
anything and reduce the # of freq transitions.  I suppose we could
opt-in to this behavior on a per-gpu basis..

BR,
-R

On Wed, Jul 28, 2021 at 5:35 PM Caleb Connolly
<caleb.connolly@linaro.org> wrote:
>
> Hi Rob,
>
> This series causes a fatal crash on my Oneplus 6, the device goes to
> Qualcomm crashdump mode shortly after reaching UI with the following errors:
>
> https://paste.ubuntu.com/p/HvjmzZYtgw/
>
> I did a git bisect and the patch ("drm/msm: Devfreq tuning") seems to be
> the cause of the crash, reverting it resolves the issue.
>
>
> On 28/07/2021 21:52, Rob Clark wrote:
> > Hi Dave & Daniel,
> >
> > An early pull for v5.15 (there'll be more coming in a week or two),
> > consisting of the drm/scheduler conversion and a couple other small
> > series that one was based one.  Mostly sending this now because IIUC
> > danvet wanted it in drm-next so he could rebase on it.  (Daniel, if
> > you disagree then speak up, and I'll instead include this in the main
> > pull request once that is ready.)
> >
> > This also has a core patch to drop drm_gem_object_put_locked() now
> > that the last use of it is removed.
> >
> > The following changes since commit ff1176468d368232b684f75e82563369208bc371:
> >
> >    Linux 5.14-rc3 (2021-07-25 15:35:14 -0700)
> >
> > are available in the Git repository at:
> >
> >    https://gitlab.freedesktop.org/drm/msm.git drm-msm-next-2021-07-28
> >
> > for you to fetch changes up to 4541e4f2225c30b0e9442be9eb2fb8b7086cdd1f:
> >
> >    drm/msm/gem: Mark active before pinning (2021-07-28 09:19:00 -0700)
> >
> > ----------------------------------------------------------------
> > Rob Clark (18):
> >        drm/msm: Let fences read directly from memptrs
> >        drm/msm: Signal fences sooner
> >        drm/msm: Split out devfreq handling
> >        drm/msm: Split out get_freq() helper
> >        drm/msm: Devfreq tuning
> >        drm/msm: Docs and misc cleanup
> >        drm/msm: Small submitqueue creation cleanup
> >        drm/msm: drop drm_gem_object_put_locked()
> >        drm: Drop drm_gem_object_put_locked()
> >        drm/msm/submit: Simplify out-fence-fd handling
> >        drm/msm: Consolidate submit bo state
> >        drm/msm: Track "seqno" fences by idr
> >        drm/msm: Return ERR_PTR() from submit_create()
> >        drm/msm: Conversion to drm scheduler
> >        drm/msm: Drop submit bo_list
> >        drm/msm: Drop struct_mutex in submit path
> >        drm/msm: Utilize gpu scheduler priorities
> >        drm/msm/gem: Mark active before pinning
> >
> >   drivers/gpu/drm/drm_gem.c                   |  22 --
> >   drivers/gpu/drm/msm/Kconfig                 |   1 +
> >   drivers/gpu/drm/msm/Makefile                |   1 +
> >   drivers/gpu/drm/msm/adreno/a5xx_debugfs.c   |   4 +-
> >   drivers/gpu/drm/msm/adreno/a5xx_gpu.c       |   6 +-
> >   drivers/gpu/drm/msm/adreno/a5xx_power.c     |   2 +-
> >   drivers/gpu/drm/msm/adreno/a5xx_preempt.c   |   7 +-
> >   drivers/gpu/drm/msm/adreno/a6xx_gmu.c       |  12 +-
> >   drivers/gpu/drm/msm/adreno/a6xx_gpu.c       |   6 +-
> >   drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c |   4 +-
> >   drivers/gpu/drm/msm/adreno/adreno_gpu.c     |   6 +-
> >   drivers/gpu/drm/msm/msm_drv.c               |  30 ++-
> >   drivers/gpu/drm/msm/msm_fence.c             |  53 +----
> >   drivers/gpu/drm/msm/msm_fence.h             |  44 +++-
> >   drivers/gpu/drm/msm/msm_gem.c               |  94 +-------
> >   drivers/gpu/drm/msm/msm_gem.h               |  47 ++--
> >   drivers/gpu/drm/msm/msm_gem_submit.c        | 344 +++++++++++++++++-----------
> >   drivers/gpu/drm/msm/msm_gpu.c               | 220 ++++--------------
> >   drivers/gpu/drm/msm/msm_gpu.h               | 139 ++++++++++-
> >   drivers/gpu/drm/msm/msm_gpu_devfreq.c       | 203 ++++++++++++++++
> >   drivers/gpu/drm/msm/msm_rd.c                |   6 +-
> >   drivers/gpu/drm/msm/msm_ringbuffer.c        |  69 +++++-
> >   drivers/gpu/drm/msm/msm_ringbuffer.h        |  12 +
> >   drivers/gpu/drm/msm/msm_submitqueue.c       |  53 +++--
> >   include/drm/drm_gem.h                       |   2 -
> >   include/uapi/drm/msm_drm.h                  |  14 +-
> >   26 files changed, 865 insertions(+), 536 deletions(-)
> >   create mode 100644 drivers/gpu/drm/msm/msm_gpu_devfreq.c
> >
>
> --
> Kind Regards,
> Caleb (they/them)

WARNING: multiple messages have this Message-ID (diff)
From: Rob Clark <robdclark@gmail.com>
To: Caleb Connolly <caleb.connolly@linaro.org>,
	Jordan Crouse <jordan@cosmicpenguin.net>
Cc: Abhinav Kumar <abhinavk@codeaurora.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	freedreno <freedreno@lists.freedesktop.org>
Subject: Re: [early pull] drm/msm: drm-msm-next-2021-07-28 for v5.15
Date: Wed, 28 Jul 2021 18:02:28 -0700	[thread overview]
Message-ID: <CAF6AEGuJjH94s0ymARtEMUo2tBuaacx7e0nqOj7_j2SQQcUa9Q@mail.gmail.com> (raw)
In-Reply-To: <7553f3cd-61c8-3ece-14ec-6c0cf4ae0296@linaro.org>

Jordan, any idea if more frequent frequency changes would for some
reason make a630 grumpy?  I was expecting it should be somewhat
similar to a618 (same GMU fw, etc).  The main result of that patch
should be clamping to min freq when gpu goes idle, and the toggling
back to devfreq provided freq on idle->active transition.  So there
might be more frequent freq transitions.

Caleb, I don't suppose you could somehow delay starting UI and get
some traces?  Something along the lines of:

  localhost ~ # cd /sys/kernel/debug/tracing/
  localhost /sys/kernel/debug/tracing # echo 1 > events/drm_msm_gpu/enable
  localhost /sys/kernel/debug/tracing # echo 1 > tracing_on
  localhost /sys/kernel/debug/tracing # cat trace_pipe

Does adding an 'if (1) return' at the top of msm_devfreq_idle() help?
That should bypass the clamping to min freq when the GPU isn't doing
anything and reduce the # of freq transitions.  I suppose we could
opt-in to this behavior on a per-gpu basis..

BR,
-R

On Wed, Jul 28, 2021 at 5:35 PM Caleb Connolly
<caleb.connolly@linaro.org> wrote:
>
> Hi Rob,
>
> This series causes a fatal crash on my Oneplus 6, the device goes to
> Qualcomm crashdump mode shortly after reaching UI with the following errors:
>
> https://paste.ubuntu.com/p/HvjmzZYtgw/
>
> I did a git bisect and the patch ("drm/msm: Devfreq tuning") seems to be
> the cause of the crash, reverting it resolves the issue.
>
>
> On 28/07/2021 21:52, Rob Clark wrote:
> > Hi Dave & Daniel,
> >
> > An early pull for v5.15 (there'll be more coming in a week or two),
> > consisting of the drm/scheduler conversion and a couple other small
> > series that one was based one.  Mostly sending this now because IIUC
> > danvet wanted it in drm-next so he could rebase on it.  (Daniel, if
> > you disagree then speak up, and I'll instead include this in the main
> > pull request once that is ready.)
> >
> > This also has a core patch to drop drm_gem_object_put_locked() now
> > that the last use of it is removed.
> >
> > The following changes since commit ff1176468d368232b684f75e82563369208bc371:
> >
> >    Linux 5.14-rc3 (2021-07-25 15:35:14 -0700)
> >
> > are available in the Git repository at:
> >
> >    https://gitlab.freedesktop.org/drm/msm.git drm-msm-next-2021-07-28
> >
> > for you to fetch changes up to 4541e4f2225c30b0e9442be9eb2fb8b7086cdd1f:
> >
> >    drm/msm/gem: Mark active before pinning (2021-07-28 09:19:00 -0700)
> >
> > ----------------------------------------------------------------
> > Rob Clark (18):
> >        drm/msm: Let fences read directly from memptrs
> >        drm/msm: Signal fences sooner
> >        drm/msm: Split out devfreq handling
> >        drm/msm: Split out get_freq() helper
> >        drm/msm: Devfreq tuning
> >        drm/msm: Docs and misc cleanup
> >        drm/msm: Small submitqueue creation cleanup
> >        drm/msm: drop drm_gem_object_put_locked()
> >        drm: Drop drm_gem_object_put_locked()
> >        drm/msm/submit: Simplify out-fence-fd handling
> >        drm/msm: Consolidate submit bo state
> >        drm/msm: Track "seqno" fences by idr
> >        drm/msm: Return ERR_PTR() from submit_create()
> >        drm/msm: Conversion to drm scheduler
> >        drm/msm: Drop submit bo_list
> >        drm/msm: Drop struct_mutex in submit path
> >        drm/msm: Utilize gpu scheduler priorities
> >        drm/msm/gem: Mark active before pinning
> >
> >   drivers/gpu/drm/drm_gem.c                   |  22 --
> >   drivers/gpu/drm/msm/Kconfig                 |   1 +
> >   drivers/gpu/drm/msm/Makefile                |   1 +
> >   drivers/gpu/drm/msm/adreno/a5xx_debugfs.c   |   4 +-
> >   drivers/gpu/drm/msm/adreno/a5xx_gpu.c       |   6 +-
> >   drivers/gpu/drm/msm/adreno/a5xx_power.c     |   2 +-
> >   drivers/gpu/drm/msm/adreno/a5xx_preempt.c   |   7 +-
> >   drivers/gpu/drm/msm/adreno/a6xx_gmu.c       |  12 +-
> >   drivers/gpu/drm/msm/adreno/a6xx_gpu.c       |   6 +-
> >   drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c |   4 +-
> >   drivers/gpu/drm/msm/adreno/adreno_gpu.c     |   6 +-
> >   drivers/gpu/drm/msm/msm_drv.c               |  30 ++-
> >   drivers/gpu/drm/msm/msm_fence.c             |  53 +----
> >   drivers/gpu/drm/msm/msm_fence.h             |  44 +++-
> >   drivers/gpu/drm/msm/msm_gem.c               |  94 +-------
> >   drivers/gpu/drm/msm/msm_gem.h               |  47 ++--
> >   drivers/gpu/drm/msm/msm_gem_submit.c        | 344 +++++++++++++++++-----------
> >   drivers/gpu/drm/msm/msm_gpu.c               | 220 ++++--------------
> >   drivers/gpu/drm/msm/msm_gpu.h               | 139 ++++++++++-
> >   drivers/gpu/drm/msm/msm_gpu_devfreq.c       | 203 ++++++++++++++++
> >   drivers/gpu/drm/msm/msm_rd.c                |   6 +-
> >   drivers/gpu/drm/msm/msm_ringbuffer.c        |  69 +++++-
> >   drivers/gpu/drm/msm/msm_ringbuffer.h        |  12 +
> >   drivers/gpu/drm/msm/msm_submitqueue.c       |  53 +++--
> >   include/drm/drm_gem.h                       |   2 -
> >   include/uapi/drm/msm_drm.h                  |  14 +-
> >   26 files changed, 865 insertions(+), 536 deletions(-)
> >   create mode 100644 drivers/gpu/drm/msm/msm_gpu_devfreq.c
> >
>
> --
> Kind Regards,
> Caleb (they/them)

  reply	other threads:[~2021-07-29  0:58 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <SKuAxGshCZFzlguCiEJOU0kAFCJ9WDGK_qCmPESnrghqei0-VIp4DD5vL58OEJCq2B-AkvF1az0EedzkGjSNLQ==@protonmail.internalid>
2021-07-28 20:52 ` [early pull] drm/msm: drm-msm-next-2021-07-28 for v5.15 Rob Clark
2021-07-28 20:52   ` Rob Clark
2021-07-29  0:35   ` Caleb Connolly
2021-07-29  0:35     ` Caleb Connolly
2021-07-29  1:02     ` Rob Clark [this message]
2021-07-29  1:02       ` Rob Clark
2021-07-29  2:18       ` Caleb Connolly
2021-07-29  2:18         ` Caleb Connolly
2021-07-29  2:50         ` Rob Clark
2021-07-29  2:50           ` Rob Clark
2021-07-29  7:13   ` Daniel Vetter
2021-07-29  7:13     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAF6AEGuJjH94s0ymARtEMUo2tBuaacx7e0nqOj7_j2SQQcUa9Q@mail.gmail.com \
    --to=robdclark@gmail.com \
    --cc=abhinavk@codeaurora.org \
    --cc=airlied@gmail.com \
    --cc=caleb.connolly@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=jordan@cosmicpenguin.net \
    --cc=linux-arm-msm@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.