[PATCHi v3] drm/mediatek: adjust rdma fifo threshold calculate formula
Change since v2: - add more commit message Yongqiang Niu (1): drm/mediatek: adjust rdma fifo threshold calculate formula drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 1.8.1.1.dirty
[PATCH v3] drm/mediatek: adjust rdma fifo threshold calculate formula
the orginal formula will caused rdma fifo threshold config overflow and no one could come out a solution for all SoC, set threshold to 70% of max fifo size to make sure it will not overflow, and 70% is a empirical vlaue Signed-off-by: Yongqiang Niu --- drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c index f123fc0..f1f6a2e 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c @@ -164,10 +164,10 @@ void mtk_rdma_config(struct device *dev, unsigned int width, /* * Enable FIFO underflow since DSI and DPI can't be blocked. * Keep the FIFO pseudo size reset default of 8 KiB. Set the -* output threshold to 6 microseconds with 7/6 overhead to -* account for blanking, and with a pixel depth of 4 bytes: +* output threshold to 70% of max fifo size to make sure the +* threhold will not overflow */ - threshold = width * height * vrefresh * 4 * 7 / 100; + threshold = rdma_fifo_size * 7 / 10; reg = RDMA_FIFO_UNDERFLOW_EN | RDMA_FIFO_PSEUDO_SIZE(rdma_fifo_size) | RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold); -- 1.8.1.1.dirty
Re: [PATCH v3 2/7] drm/msm/dsi: add three helper functions
Hi Dmitry,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on next-20210709]
[cannot apply to v5.13]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/drm-msm-dpu-add-support-for-independent-DSI-config/20210711-062150
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
1e16624d7b4376797ede36e3c955375cf0f23298
config: arm-randconfig-r016-20210711 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project
8d69635ed9ecf36fd0ca85906bfde17949671cbe)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
#
https://github.com/0day-ci/linux/commit/d60158cc75b16689ac121290b562160916b57833
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Dmitry-Baryshkov/drm-msm-dpu-add-support-for-independent-DSI-config/20210711-062150
git checkout d60158cc75b16689ac121290b562160916b57833
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir
ARCH=arm SHELL=/bin/bash drivers/gpu/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All error/warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/msm/adreno/adreno_device.c:9:
In file included from drivers/gpu/drm/msm/adreno/adreno_gpu.h:15:
In file included from drivers/gpu/drm/msm/msm_gpu.h:16:
>> drivers/gpu/drm/msm/msm_drv.h:382:14: error: expected ';' after return
>> statement
return false
^
;
>> drivers/gpu/drm/msm/msm_drv.h:380:6: warning: no previous prototype for
>> function 'msm_dsi_is_master_dsi' [-Wmissing-prototypes]
bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
^
drivers/gpu/drm/msm/msm_drv.h:380:1: note: declare 'static' if the function
is not intended to be used outside of this translation unit
bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
^
static
1 warning and 1 error generated.
--
In file included from drivers/gpu/drm/msm/dp/dp_display.c:14:
>> drivers/gpu/drm/msm/msm_drv.h:382:14: error: expected ';' after return
>> statement
return false
^
;
>> drivers/gpu/drm/msm/msm_drv.h:380:6: warning: no previous prototype for
>> function 'msm_dsi_is_master_dsi' [-Wmissing-prototypes]
bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
^
drivers/gpu/drm/msm/msm_drv.h:380:1: note: declare 'static' if the function
is not intended to be used outside of this translation unit
bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
^
static
drivers/gpu/drm/msm/dp/dp_display.c:1017:21: warning: variable 'drm' set but
not used [-Wunused-but-set-variable]
struct drm_device *drm;
^
2 warnings and 1 error generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for QCOM_SCM
Depends on (ARM || ARM64) && HAVE_ARM_SMCCC
Selected by
- ARM_QCOM_SPM_CPUIDLE && CPU_IDLE && (ARM || ARM64) && (ARCH_QCOM ||
COMPILE_TEST && !ARM64 && MMU
vim +382 drivers/gpu/drm/msm/msm_drv.h
339
340 struct msm_edp;
341 void __init msm_edp_register(void);
342 void __exit msm_edp_unregister(void);
343 int msm_edp_modeset_init(struct msm_edp *edp, struct drm_device *dev,
344 struct drm_encoder *encoder);
345
346 struct msm_dsi;
347 #ifdef CONFIG_DRM_MSM_DSI
348 void __init msm_dsi_register(void);
349 void __exit msm_dsi_unregister(void);
350 int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device
*dev,
351 struct drm_encoder *encoder);
352 void msm_dsi_snapshot(struct msm_disp_state *disp_state, struct msm_dsi
*msm_dsi);
353 bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi);
354 bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi);
355 bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi);
356 #else
357 static inline void __init msm_dsi_register(void)
358 {
359 }
360 static inline void __exit msm_dsi_unregister(void)
361 {
362 }
363 static inline int msm_dsi_modeset_init(struct msm_dsi *msm_dsi,
364 struct drm_device *dev,
365 struct drm_encoder *encoder)
366 {
367 return -EINVAL;
[PATCH 5/5] Revert "drm/i915: Skip over MI_NOOP when parsing"
This reverts a6c5e2aea704 ("drm/i915: Skip over MI_NOOP when parsing").
It complicates the batch parsing code a bit and increases indentation
for no reason other than fast-skipping a command that userspace uses
only rarely. Sure, there may be IGT tests that fill batches with NOOPs
but that's not a case we should optimize for in the kernel. We should
optimize for code clarity instead.
Signed-off-by: Jason Ekstrand
Reviewed-by: Jon Bloomfield
Acked-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 67 +-
1 file changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 00ec618d01590..322f4d5955a4f 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1470,42 +1470,43 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
* space. Parsing should be faster in some cases this way.
*/
batch_end = cmd + batch_length / sizeof(*batch_end);
- while (*cmd != MI_BATCH_BUFFER_END) {
- u32 length = 1;
-
- if (*cmd != MI_NOOP) { /* MI_NOOP == 0 */
- desc = find_cmd(engine, *cmd, desc, &default_desc);
- if (!desc) {
- DRM_DEBUG("CMD: Unrecognized command:
0x%08X\n", *cmd);
- ret = -EINVAL;
- break;
- }
+ do {
+ u32 length;
- if (desc->flags & CMD_DESC_FIXED)
- length = desc->length.fixed;
- else
- length = (*cmd & desc->length.mask) +
LENGTH_BIAS;
+ if (*cmd == MI_BATCH_BUFFER_END)
+ break;
- if ((batch_end - cmd) < length) {
- DRM_DEBUG("CMD: Command length exceeds batch
length: 0x%08X length=%u batchlen=%td\n",
- *cmd,
- length,
- batch_end - cmd);
- ret = -EINVAL;
- break;
- }
+ desc = find_cmd(engine, *cmd, desc, &default_desc);
+ if (!desc) {
+ DRM_DEBUG("CMD: Unrecognized command: 0x%08X\n", *cmd);
+ ret = -EINVAL;
+ break;
+ }
- if (!check_cmd(engine, desc, cmd, length)) {
- ret = -EACCES;
- break;
- }
+ if (desc->flags & CMD_DESC_FIXED)
+ length = desc->length.fixed;
+ else
+ length = (*cmd & desc->length.mask) + LENGTH_BIAS;
- if (cmd_desc_is(desc, MI_BATCH_BUFFER_START)) {
- ret = check_bbstart(cmd, offset, length,
batch_length,
- batch_addr, shadow_addr,
- jump_whitelist);
- break;
- }
+ if ((batch_end - cmd) < length) {
+ DRM_DEBUG("CMD: Command length exceeds batch length:
0x%08X length=%u batchlen=%td\n",
+ *cmd,
+ length,
+ batch_end - cmd);
+ ret = -EINVAL;
+ break;
+ }
+
+ if (!check_cmd(engine, desc, cmd, length)) {
+ ret = -EACCES;
+ break;
+ }
+
+ if (cmd_desc_is(desc, MI_BATCH_BUFFER_START)) {
+ ret = check_bbstart(cmd, offset, length, batch_length,
+ batch_addr, shadow_addr,
+ jump_whitelist);
+ break;
}
if (!IS_ERR_OR_NULL(jump_whitelist))
@@ -1518,7 +1519,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs
*engine,
ret = -EINVAL;
break;
}
- }
+ } while (1);
if (trampoline) {
/*
--
2.31.1
[PATCH 4/5] drm/i915: Drop error handling from dma_fence_work
Asynchronous command parsing was the only thing which ever returned a
non-zero error. With that gone, we can drop the error handling from
dma_fence_work.
Signed-off-by: Jason Ekstrand
Reviewed-by: Jon Bloomfield
Acked-by: Daniel Vetter
---
drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 4 +---
drivers/gpu/drm/i915/i915_sw_fence_work.c | 5 +
drivers/gpu/drm/i915/i915_sw_fence_work.h | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 3 +--
4 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
index daf9284ef1f54..f0435c6feb68b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
@@ -24,13 +24,11 @@ static void __do_clflush(struct drm_i915_gem_object *obj)
i915_gem_object_flush_frontbuffer(obj, ORIGIN_CPU);
}
-static int clflush_work(struct dma_fence_work *base)
+static void clflush_work(struct dma_fence_work *base)
{
struct clflush *clflush = container_of(base, typeof(*clflush), base);
__do_clflush(clflush->obj);
-
- return 0;
}
static void clflush_release(struct dma_fence_work *base)
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c
b/drivers/gpu/drm/i915/i915_sw_fence_work.c
index a3a81bb8f2c36..5b33ef23d54c9 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
@@ -16,11 +16,8 @@ static void fence_complete(struct dma_fence_work *f)
static void fence_work(struct work_struct *work)
{
struct dma_fence_work *f = container_of(work, typeof(*f), work);
- int err;
- err = f->ops->work(f);
- if (err)
- dma_fence_set_error(&f->dma, err);
+ f->ops->work(f);
fence_complete(f);
dma_fence_put(&f->dma);
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.h
b/drivers/gpu/drm/i915/i915_sw_fence_work.h
index 2c409f11c5c59..d56806918d131 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence_work.h
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.h
@@ -17,7 +17,7 @@ struct dma_fence_work;
struct dma_fence_work_ops {
const char *name;
- int (*work)(struct dma_fence_work *f);
+ void (*work)(struct dma_fence_work *f);
void (*release)(struct dma_fence_work *f);
};
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 0f227f28b2802..5b9dce0f443b0 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -300,14 +300,13 @@ struct i915_vma_work {
unsigned int flags;
};
-static int __vma_bind(struct dma_fence_work *work)
+static void __vma_bind(struct dma_fence_work *work)
{
struct i915_vma_work *vw = container_of(work, typeof(*vw), base);
struct i915_vma *vma = vw->vma;
vma->ops->bind_vma(vw->vm, &vw->stash,
vma, vw->cache_level, vw->flags);
- return 0;
}
static void __vma_release(struct dma_fence_work *work)
--
2.31.1
[PATCH 1/5] drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser"
This reverts 686c7c35abc2 ("drm/i915/gem: Asynchronous cmdparser"). The
justification for this commit in the git history was a vague comment
about getting it out from under the struct_mutex. While this may
improve perf for some workloads on Gen7 platforms where we rely on the
command parser for features such as indirect rendering, no numbers were
provided to prove such an improvement. It claims to closed two
gitlab/bugzilla issues but with no explanation whatsoever as to why or
what bug it's fixing.
Meanwhile, by moving command parsing off to an async callback, it leaves
us with a problem of what to do on error. When things were synchronous,
EXECBUFFER2 would fail with an error code if parsing failed. When
moving it to async, we needed another way to handle that error and the
solution employed was to set an error on the dma_fence and then trust
that said error gets propagated to the client eventually. Moving back
to synchronous will help us untangle the fence error propagation mess.
This also reverts most of 0edbb9ba1bfe ("drm/i915: Move cmd parser
pinning to execbuffer") which is a refactor of some of our allocation
paths for asynchronous parsing. Now that everything is synchronous, we
don't need it.
v2 (Daniel Vetter):
- Add stabel Cc and Fixes tag
Signed-off-by: Jason Ekstrand
Cc: # v5.6+
Fixes: 9e31c1fe45d5 ("drm/i915: Propagate errors on awaiting already signaled
fences")
Cc: Maarten Lankhorst
Reviewed-by: Jon Bloomfield
Acked-by: Daniel Vetter
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 227 +-
.../i915/gem/selftests/i915_gem_execbuffer.c | 4 +
drivers/gpu/drm/i915/i915_cmd_parser.c| 132 +-
drivers/gpu/drm/i915/i915_drv.h | 7 +-
4 files changed, 91 insertions(+), 279 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 5ea8b4e23e428..1ed7475de454d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -25,10 +25,8 @@
#include "i915_gem_clflush.h"
#include "i915_gem_context.h"
#include "i915_gem_ioctls.h"
-#include "i915_sw_fence_work.h"
#include "i915_trace.h"
#include "i915_user_extensions.h"
-#include "i915_memcpy.h"
struct eb_vma {
struct i915_vma *vma;
@@ -1471,6 +1469,10 @@ static u32 *reloc_gpu(struct i915_execbuffer *eb,
int err;
struct intel_engine_cs *engine = eb->engine;
+ /* If we need to copy for the cmdparser, we will stall anyway */
+ if (eb_use_cmdparser(eb))
+ return ERR_PTR(-EWOULDBLOCK);
+
if (!reloc_can_use_engine(engine)) {
engine = engine->gt->engine_class[COPY_ENGINE_CLASS][0];
if (!engine)
@@ -2385,217 +2387,6 @@ shadow_batch_pin(struct i915_execbuffer *eb,
return vma;
}
-struct eb_parse_work {
- struct dma_fence_work base;
- struct intel_engine_cs *engine;
- struct i915_vma *batch;
- struct i915_vma *shadow;
- struct i915_vma *trampoline;
- unsigned long batch_offset;
- unsigned long batch_length;
- unsigned long *jump_whitelist;
- const void *batch_map;
- void *shadow_map;
-};
-
-static int __eb_parse(struct dma_fence_work *work)
-{
- struct eb_parse_work *pw = container_of(work, typeof(*pw), base);
- int ret;
- bool cookie;
-
- cookie = dma_fence_begin_signalling();
- ret = intel_engine_cmd_parser(pw->engine,
- pw->batch,
- pw->batch_offset,
- pw->batch_length,
- pw->shadow,
- pw->jump_whitelist,
- pw->shadow_map,
- pw->batch_map);
- dma_fence_end_signalling(cookie);
-
- return ret;
-}
-
-static void __eb_parse_release(struct dma_fence_work *work)
-{
- struct eb_parse_work *pw = container_of(work, typeof(*pw), base);
-
- if (!IS_ERR_OR_NULL(pw->jump_whitelist))
- kfree(pw->jump_whitelist);
-
- if (pw->batch_map)
- i915_gem_object_unpin_map(pw->batch->obj);
- else
- i915_gem_object_unpin_pages(pw->batch->obj);
-
- i915_gem_object_unpin_map(pw->shadow->obj);
-
- if (pw->trampoline)
- i915_active_release(&pw->trampoline->active);
- i915_active_release(&pw->shadow->active);
- i915_active_release(&pw->batch->active);
-}
-
-static const struct dma_fence_work_ops eb_parse_ops = {
- .name = "eb_parse",
- .work = __eb_parse,
- .release = __eb_parse_release,
-};
-
-static inline int
-__parser_mark_active(struct i915_vma *vma,
-struct intel_timeline *tl,
-struct dma_fence *fence)
-{
- st
[PATCH 2/5] Revert "drm/i915: Propagate errors on awaiting already signaled fences"
This reverts commit 9e31c1fe45d555a948ff66f1f0e3fe1f83ca63f7. Ever
since that commit, we've been having issues where a hang in one client
can propagate to another. In particular, a hang in an app can propagate
to the X server which causes the whole desktop to lock up.
Error propagation along fences sound like a good idea, but as your bug
shows, surprising consequences, since propagating errors across security
boundaries is not a good thing.
What we do have is track the hangs on the ctx, and report information to
userspace using RESET_STATS. That's how arb_robustness works. Also, if my
understanding is still correct, the EIO from execbuf is when your context
is banned (because not recoverable or too many hangs). And in all these
cases it's up to userspace to figure out what is all impacted and should
be reported to the application, that's not on the kernel to guess and
automatically propagate.
What's more, we're also building more features on top of ctx error
reporting with RESET_STATS ioctl: Encrypted buffers use the same, and the
userspace fence wait also relies on that mechanism. So it is the path
going forward for reporting gpu hangs and resets to userspace.
So all together that's why I think we should just bury this idea again as
not quite the direction we want to go to, hence why I think the revert is
the right option here.
For backporters: Please note that you _must_ have a backport of
https://lore.kernel.org/dri-devel/20210602164149.391653-2-ja...@jlekstrand.net/
for otherwise backporting just this patch opens up a security bug.
v2: Augment commit message. Also restore Jason's sob that I
accidentally lost.
v3: Add a note for backporters
Signed-off-by: Jason Ekstrand
Reported-by: Marcin Slusarz
Cc: # v5.6+
Cc: Jason Ekstrand
Cc: Marcin Slusarz
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3080
Fixes: 9e31c1fe45d5 ("drm/i915: Propagate errors on awaiting already signaled
fences")
Acked-by: Daniel Vetter
Reviewed-by: Jon Bloomfield
---
drivers/gpu/drm/i915/i915_request.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index 86b4c9f2613d5..09ebea9a0090a 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1399,10 +1399,8 @@ i915_request_await_execution(struct i915_request *rq,
do {
fence = *child++;
- if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
- i915_sw_fence_set_error_once(&rq->submit, fence->error);
+ if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
continue;
- }
if (fence->context == rq->fence.context)
continue;
@@ -1499,10 +1497,8 @@ i915_request_await_dma_fence(struct i915_request *rq,
struct dma_fence *fence)
do {
fence = *child++;
- if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
- i915_sw_fence_set_error_once(&rq->submit, fence->error);
+ if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
continue;
- }
/*
* Requests on the same timeline are explicitly ordered, along
--
2.31.1
[PATCH 3/5] drm/i915: Remove allow_alloc from i915_gem_object_get_sg*
This reverts the rest of 0edbb9ba1bfe ("drm/i915: Move cmd parser
pinning to execbuffer"). Now that the only user of i915_gem_object_get_sg
without allow_alloc has been removed, we can drop the parameter. This
portion of the revert was broken into its own patch to aid review.
Signed-off-by: Jason Ekstrand
Cc: Maarten Lankhorst
Reviewed-by: Jon Bloomfield
Acked-by: Daniel Vetter
---
drivers/gpu/drm/i915/gem/i915_gem_object.h | 10 +-
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 20
drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 2 +-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +-
4 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index d423d8cac4f26..1603ed90d9c13 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -341,22 +341,22 @@ struct scatterlist *
__i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
struct i915_gem_object_page_iter *iter,
unsigned int n,
-unsigned int *offset, bool allow_alloc, bool dma);
+unsigned int *offset, bool dma);
static inline struct scatterlist *
i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
unsigned int n,
- unsigned int *offset, bool allow_alloc)
+ unsigned int *offset)
{
- return __i915_gem_object_get_sg(obj, &obj->mm.get_page, n, offset,
allow_alloc, false);
+ return __i915_gem_object_get_sg(obj, &obj->mm.get_page, n, offset,
false);
}
static inline struct scatterlist *
i915_gem_object_get_sg_dma(struct drm_i915_gem_object *obj,
unsigned int n,
- unsigned int *offset, bool allow_alloc)
+ unsigned int *offset)
{
- return __i915_gem_object_get_sg(obj, &obj->mm.get_dma_page, n, offset,
allow_alloc, true);
+ return __i915_gem_object_get_sg(obj, &obj->mm.get_dma_page, n, offset,
true);
}
struct page *
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index f2f850e31b8ed..40cad2655b637 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -467,7 +467,7 @@ __i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
struct i915_gem_object_page_iter *iter,
unsigned int n,
unsigned int *offset,
-bool allow_alloc, bool dma)
+bool dma)
{
struct scatterlist *sg;
unsigned int idx, count;
@@ -489,9 +489,6 @@ __i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
if (n < READ_ONCE(iter->sg_idx))
goto lookup;
- if (!allow_alloc)
- goto manual_lookup;
-
mutex_lock(&iter->lock);
/* We prefer to reuse the last sg so that repeated lookup of this
@@ -541,16 +538,7 @@ __i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
if (unlikely(n < idx)) /* insertion completed by another thread */
goto lookup;
- goto manual_walk;
-
-manual_lookup:
- idx = 0;
- sg = obj->mm.pages->sgl;
- count = __sg_page_count(sg);
-
-manual_walk:
- /*
-* In case we failed to insert the entry into the radixtree, we need
+ /* In case we failed to insert the entry into the radixtree, we need
* to look beyond the current sg.
*/
while (idx + count <= n) {
@@ -597,7 +585,7 @@ i915_gem_object_get_page(struct drm_i915_gem_object *obj,
unsigned int n)
GEM_BUG_ON(!i915_gem_object_has_struct_page(obj));
- sg = i915_gem_object_get_sg(obj, n, &offset, true);
+ sg = i915_gem_object_get_sg(obj, n, &offset);
return nth_page(sg_page(sg), offset);
}
@@ -623,7 +611,7 @@ i915_gem_object_get_dma_address_len(struct
drm_i915_gem_object *obj,
struct scatterlist *sg;
unsigned int offset;
- sg = i915_gem_object_get_sg_dma(obj, n, &offset, true);
+ sg = i915_gem_object_get_sg_dma(obj, n, &offset);
if (len)
*len = sg_dma_len(sg) - (offset << PAGE_SHIFT);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 6589411396d3f..f253b11e9e367 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -589,7 +589,7 @@ static unsigned long i915_ttm_io_mem_pfn(struct
ttm_buffer_object *bo,
GEM_WARN_ON(bo->ttm);
- sg = __i915_gem_object_get_sg(obj, &obj->ttm.get_io_page, page_offset,
&ofs, true, true);
+ sg = __i915_gem_object_get_sg(obj, &obj->ttm.get_io_page, page_offset,
&ofs, true);
return ((base + sg_dma_address(sg)) >> PAGE_SHIFT) + ofs;
}
diff
[PATCH 0/5] drm/i915: Get rid of fence error propagation (v2)
Fence error propagation is sketchy at best. Instead of explicitly handling fences which might have errors set in the code which is aware of errors, we just kick them down the line and hope that userspace knows what to do when a wait eventually fails. This is sketchy at best because most userspace isn't prepared to handle errors in those places. To make things worse, it allows errors to propagate across processes in unpredictable ways. This is causing hangs in one client to kill X11. Unfortunately, there's no quick path from here to there thanks to the fact that we're now running the command parser asynchronously and relying on fence errors for when it fails. This series first gets rid of asynchronous command parsing and then cleans up from there. There was never any real use-case for asynchronous parsing and the platforms that rely heavily on the command parser are old enough (Gen7) that, when we changed the way the command parser works, it wasn't really a change anyone was asking for anyway. I think we probably want this whole mess back-ported. I'm happy to take suggestions on the strategy there because the history there is a bit annoying and I'm not 100% sure where the Linux release cuts land. In any case, I'm happy to make a version of this series per-release if needed for Greg to back-port. v2 (Daniel Vetter): - Re-order to put the reverts first - Add ACKs from Daniel - Add better CC and Fixes tags v3 (Daniel Vetter): - Rebase on drm-tip Test-with: 20210711035204.802908-1-ja...@jlekstrand.net Cc: Daniel Vetter Cc: Jon Bloomfield Jason Ekstrand (5): drm/i915: Revert "drm/i915/gem: Asynchronous cmdparser" Revert "drm/i915: Propagate errors on awaiting already signaled fences" drm/i915: Remove allow_alloc from i915_gem_object_get_sg* drm/i915: Drop error handling from dma_fence_work Revert "drm/i915: Skip over MI_NOOP when parsing" drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 4 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 227 +- drivers/gpu/drm/i915/gem/i915_gem_object.h| 10 +- drivers/gpu/drm/i915/gem/i915_gem_pages.c | 20 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- .../i915/gem/selftests/i915_gem_execbuffer.c | 4 + drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +- drivers/gpu/drm/i915/i915_cmd_parser.c| 199 --- drivers/gpu/drm/i915/i915_drv.h | 7 +- drivers/gpu/drm/i915/i915_request.c | 8 +- drivers/gpu/drm/i915/i915_sw_fence_work.c | 5 +- drivers/gpu/drm/i915/i915_sw_fence_work.h | 2 +- drivers/gpu/drm/i915/i915_vma.c | 3 +- 13 files changed, 142 insertions(+), 351 deletions(-) -- 2.31.1
[PATCH AUTOSEL 5.10 28/37] drm/amdkfd: fix sysfs kobj leak
From: Philip Yang
[ Upstream commit dcdb4d904b4bd3078fe8d4d24b1658560d6078ef ]
3 cases of kobj leak, which causes memory leak:
kobj_type must have release() method to free memory from release
callback. Don't need NULL default_attrs to init kobj.
sysfs files created under kobj_status should be removed with kobj_status
as parent kobject.
Remove queue sysfs files when releasing queue from process MMU notifier
release callback.
Signed-off-by: Philip Yang
Reviewed-by: Felix Kuehling
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 14 ++
.../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 1 +
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 65803e153a22..d243e60c6eef 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -452,13 +452,9 @@ static const struct sysfs_ops procfs_stats_ops = {
.show = kfd_procfs_stats_show,
};
-static struct attribute *procfs_stats_attrs[] = {
- NULL
-};
-
static struct kobj_type procfs_stats_type = {
.sysfs_ops = &procfs_stats_ops,
- .default_attrs = procfs_stats_attrs,
+ .release = kfd_procfs_kobj_release,
};
int kfd_procfs_add_queue(struct queue *q)
@@ -973,9 +969,11 @@ static void kfd_process_wq_release(struct work_struct
*work)
list_for_each_entry(pdd, &p->per_device_data, per_device_list) {
sysfs_remove_file(p->kobj, &pdd->attr_vram);
sysfs_remove_file(p->kobj, &pdd->attr_sdma);
- sysfs_remove_file(p->kobj, &pdd->attr_evict);
- if (pdd->dev->kfd2kgd->get_cu_occupancy != NULL)
- sysfs_remove_file(p->kobj,
&pdd->attr_cu_occupancy);
+
+ sysfs_remove_file(pdd->kobj_stats, &pdd->attr_evict);
+ if (pdd->dev->kfd2kgd->get_cu_occupancy)
+ sysfs_remove_file(pdd->kobj_stats,
+ &pdd->attr_cu_occupancy);
kobject_del(pdd->kobj_stats);
kobject_put(pdd->kobj_stats);
pdd->kobj_stats = NULL;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index eb1635ac8988..43c07ac2c6fc 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -153,6 +153,7 @@ void pqm_uninit(struct process_queue_manager *pqm)
if (pqn->q && pqn->q->gws)
amdgpu_amdkfd_remove_gws_from_process(pqm->process->kgd_process_info,
pqn->q->gws);
+ kfd_procfs_del_queue(pqn->q);
uninit_queue(pqn->q);
list_del(&pqn->process_queue_list);
kfree(pqn);
--
2.30.2
[PATCH AUTOSEL 5.10 24/37] drm/gma500: Add the missed drm_gem_object_put() in psb_user_framebuffer_create()
From: Jing Xiangfeng
[ Upstream commit cd8f318fbd266b127ffc93cc4c1eaf9a5196fafb ]
psb_user_framebuffer_create() misses to call drm_gem_object_put() in an
error path. Add the missed function call to fix it.
Signed-off-by: Jing Xiangfeng
Signed-off-by: Daniel Vetter
Link:
https://patchwork.freedesktop.org/patch/msgid/20210629115956.15160-1-jingxiangf...@huawei.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/gma500/framebuffer.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c
b/drivers/gpu/drm/gma500/framebuffer.c
index 54d9876b5305..6ef4ea07d1bb 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -435,6 +435,7 @@ static struct drm_framebuffer *psb_user_framebuffer_create
const struct drm_mode_fb_cmd2 *cmd)
{
struct drm_gem_object *obj;
+ struct drm_framebuffer *fb;
/*
* Find the GEM object and thus the gtt range object that is
@@ -445,7 +446,11 @@ static struct drm_framebuffer *psb_user_framebuffer_create
return ERR_PTR(-ENOENT);
/* Let the core code do all the work */
- return psb_framebuffer_create(dev, cmd, obj);
+ fb = psb_framebuffer_create(dev, cmd, obj);
+ if (IS_ERR(fb))
+ drm_gem_object_put(obj);
+
+ return fb;
}
static int psbfb_probe(struct drm_fb_helper *fb_helper,
--
2.30.2
[PATCH AUTOSEL 5.12 31/43] drm/amdgpu: fix Navi1x tcp power gating hang when issuing lightweight invalidaiton
From: Evan Quan
[ Upstream commit 9c26ddb1c5b6e30c6bca48b8ad9205d96efe93d0 ]
Fix TCP hang when a lightweight invalidation happens on Navi1x.
Signed-off-by: Evan Quan
Reviewed-by: Lijo Lazar
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 95 ++
1 file changed, 95 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 2342c5d216f9..5c40912b51d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -7744,6 +7744,97 @@ static void
gfx_v10_0_update_fine_grain_clock_gating(struct amdgpu_device *adev,
}
}
+static void gfx_v10_0_apply_medium_grain_clock_gating_workaround(struct
amdgpu_device *adev)
+{
+ uint32_t reg_data = 0;
+ uint32_t reg_idx = 0;
+ uint32_t i;
+
+ const uint32_t tcp_ctrl_regs[] = {
+ mmCGTS_SA0_WGP00_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP00_CU1_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP01_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP01_CU1_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP02_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP02_CU1_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP10_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP10_CU1_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP11_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP11_CU1_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP12_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP12_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP00_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP00_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP01_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP01_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP02_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP02_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP10_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP10_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP11_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP11_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP12_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP12_CU1_TCP_CTRL_REG
+ };
+
+ const uint32_t tcp_ctrl_regs_nv12[] = {
+ mmCGTS_SA0_WGP00_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP00_CU1_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP01_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP01_CU1_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP02_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP02_CU1_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP10_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP10_CU1_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP11_CU0_TCP_CTRL_REG,
+ mmCGTS_SA0_WGP11_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP00_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP00_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP01_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP01_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP02_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP02_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP10_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP10_CU1_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP11_CU0_TCP_CTRL_REG,
+ mmCGTS_SA1_WGP11_CU1_TCP_CTRL_REG,
+ };
+
+ const uint32_t sm_ctlr_regs[] = {
+ mmCGTS_SA0_QUAD0_SM_CTRL_REG,
+ mmCGTS_SA0_QUAD1_SM_CTRL_REG,
+ mmCGTS_SA1_QUAD0_SM_CTRL_REG,
+ mmCGTS_SA1_QUAD1_SM_CTRL_REG
+ };
+
+ if (adev->asic_type == CHIP_NAVI12) {
+ for (i = 0; i < ARRAY_SIZE(tcp_ctrl_regs_nv12); i++) {
+ reg_idx =
adev->reg_offset[GC_HWIP][0][mmCGTS_SA0_WGP00_CU0_TCP_CTRL_REG_BASE_IDX] +
+ tcp_ctrl_regs_nv12[i];
+ reg_data = RREG32(reg_idx);
+ reg_data |=
CGTS_SA0_WGP00_CU0_TCP_CTRL_REG__TCPI_LS_OVERRIDE_MASK;
+ WREG32(reg_idx, reg_data);
+ }
+ } else {
+ for (i = 0; i < ARRAY_SIZE(tcp_ctrl_regs); i++) {
+ reg_idx =
adev->reg_offset[GC_HWIP][0][mmCGTS_SA0_WGP00_CU0_TCP_CTRL_REG_BASE_IDX] +
+ tcp_ctrl_regs[i];
+ reg_data = RREG32(reg_idx);
+ reg_data |=
CGTS_SA0_WGP00_CU0_TCP_CTRL_REG__TCPI_LS_OVERRIDE_MASK;
+ WREG32(reg_idx, reg_data);
+ }
+ }
+
+ for (i = 0; i < ARRAY_SIZE(sm_ctlr_regs); i++) {
+ reg_idx =
adev->reg_offset[GC_HWIP][0][mmCGTS_SA0_QUAD0_SM_CTRL_REG_BASE_IDX] +
+ sm_ctlr_regs[i];
+ reg_data = RREG32(reg_idx);
+ reg_data &= ~CGTS_SA0_QUAD0_SM_CTRL_REG__SM_MODE_MASK;
+ reg_data |= 2 << CGTS_SA0_QUAD0_SM_CTRL_REG__SM_MODE__SHIFT;
+ WREG32(reg_idx, reg_data);
+ }
+}
+
static int gfx_v10_0_update_gfx_clock_gating(struct amdgpu_device *adev,
[PATCH AUTOSEL 5.12 32/43] drm/amdkfd: fix sysfs kobj leak
From: Philip Yang
[ Upstream commit dcdb4d904b4bd3078fe8d4d24b1658560d6078ef ]
3 cases of kobj leak, which causes memory leak:
kobj_type must have release() method to free memory from release
callback. Don't need NULL default_attrs to init kobj.
sysfs files created under kobj_status should be removed with kobj_status
as parent kobject.
Remove queue sysfs files when releasing queue from process MMU notifier
release callback.
Signed-off-by: Philip Yang
Reviewed-by: Felix Kuehling
Signed-off-by: Alex Deucher
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 14 ++
.../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 1 +
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 65803e153a22..d243e60c6eef 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -452,13 +452,9 @@ static const struct sysfs_ops procfs_stats_ops = {
.show = kfd_procfs_stats_show,
};
-static struct attribute *procfs_stats_attrs[] = {
- NULL
-};
-
static struct kobj_type procfs_stats_type = {
.sysfs_ops = &procfs_stats_ops,
- .default_attrs = procfs_stats_attrs,
+ .release = kfd_procfs_kobj_release,
};
int kfd_procfs_add_queue(struct queue *q)
@@ -973,9 +969,11 @@ static void kfd_process_wq_release(struct work_struct
*work)
list_for_each_entry(pdd, &p->per_device_data, per_device_list) {
sysfs_remove_file(p->kobj, &pdd->attr_vram);
sysfs_remove_file(p->kobj, &pdd->attr_sdma);
- sysfs_remove_file(p->kobj, &pdd->attr_evict);
- if (pdd->dev->kfd2kgd->get_cu_occupancy != NULL)
- sysfs_remove_file(p->kobj,
&pdd->attr_cu_occupancy);
+
+ sysfs_remove_file(pdd->kobj_stats, &pdd->attr_evict);
+ if (pdd->dev->kfd2kgd->get_cu_occupancy)
+ sysfs_remove_file(pdd->kobj_stats,
+ &pdd->attr_cu_occupancy);
kobject_del(pdd->kobj_stats);
kobject_put(pdd->kobj_stats);
pdd->kobj_stats = NULL;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index eb1635ac8988..43c07ac2c6fc 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -153,6 +153,7 @@ void pqm_uninit(struct process_queue_manager *pqm)
if (pqn->q && pqn->q->gws)
amdgpu_amdkfd_remove_gws_from_process(pqm->process->kgd_process_info,
pqn->q->gws);
+ kfd_procfs_del_queue(pqn->q);
uninit_queue(pqn->q);
list_del(&pqn->process_queue_list);
kfree(pqn);
--
2.30.2
[PATCH AUTOSEL 5.12 25/43] drm/gma500: Add the missed drm_gem_object_put() in psb_user_framebuffer_create()
From: Jing Xiangfeng
[ Upstream commit cd8f318fbd266b127ffc93cc4c1eaf9a5196fafb ]
psb_user_framebuffer_create() misses to call drm_gem_object_put() in an
error path. Add the missed function call to fix it.
Signed-off-by: Jing Xiangfeng
Signed-off-by: Daniel Vetter
Link:
https://patchwork.freedesktop.org/patch/msgid/20210629115956.15160-1-jingxiangf...@huawei.com
Signed-off-by: Sasha Levin
---
drivers/gpu/drm/gma500/framebuffer.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c
b/drivers/gpu/drm/gma500/framebuffer.c
index ebe9dccf2d83..0b8648396fb2 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -352,6 +352,7 @@ static struct drm_framebuffer *psb_user_framebuffer_create
const struct drm_mode_fb_cmd2 *cmd)
{
struct drm_gem_object *obj;
+ struct drm_framebuffer *fb;
/*
* Find the GEM object and thus the gtt range object that is
@@ -362,7 +363,11 @@ static struct drm_framebuffer *psb_user_framebuffer_create
return ERR_PTR(-ENOENT);
/* Let the core code do all the work */
- return psb_framebuffer_create(dev, cmd, obj);
+ fb = psb_framebuffer_create(dev, cmd, obj);
+ if (IS_ERR(fb))
+ drm_gem_object_put(obj);
+
+ return fb;
}
static int psbfb_probe(struct drm_fb_helper *fb_helper,
--
2.30.2
[PATCH v3 7/7] drm/msm/kms: drop set_encoder_mode callback
set_encoder_mode callback is completely unused now. Drop it from
msm_kms_func().
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/msm_kms.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 086a2d59b8c8..9484e8b62630 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -117,9 +117,6 @@ struct msm_kms_funcs {
struct drm_encoder *encoder,
struct drm_encoder *slave_encoder,
bool is_cmd_mode);
- void (*set_encoder_mode)(struct msm_kms *kms,
-struct drm_encoder *encoder,
-bool cmd_mode);
/* cleanup: */
void (*destroy)(struct msm_kms *kms);
--
2.30.2
[PATCH v3 4/7] drm/msm/mdp5: move mdp5_encoder_set_intf_mode after msm_dsi_modeset_init
Move a call to mdp5_encoder_set_intf_mode() after
msm_dsi_modeset_init(), removing set_encoder_mode callback.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 15aed45022bc..b3b42672b2d4 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -209,13 +209,6 @@ static int mdp5_set_split_display(struct msm_kms *kms,
slave_encoder);
}
-static void mdp5_set_encoder_mode(struct msm_kms *kms,
- struct drm_encoder *encoder,
- bool cmd_mode)
-{
- mdp5_encoder_set_intf_mode(encoder, cmd_mode);
-}
-
static void mdp5_kms_destroy(struct msm_kms *kms)
{
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
@@ -287,7 +280,6 @@ static const struct mdp_kms_funcs kms_funcs = {
.get_format = mdp_get_format,
.round_pixclk= mdp5_round_pixclk,
.set_split_display = mdp5_set_split_display,
- .set_encoder_mode = mdp5_set_encoder_mode,
.destroy = mdp5_kms_destroy,
#ifdef CONFIG_DEBUG_FS
.debugfs_init= mdp5_kms_debugfs_init,
@@ -448,6 +440,9 @@ static int modeset_init_intf(struct mdp5_kms *mdp5_kms,
}
ret = msm_dsi_modeset_init(priv->dsi[dsi_id], dev, encoder);
+ if (!ret)
+ mdp5_encoder_set_intf_mode(encoder,
msm_dsi_is_cmd_mode(priv->dsi[dsi_id]));
+
break;
}
default:
--
2.30.2
[PATCH v3 6/7] drm/msm/dsi: stop calling set_encoder_mode callback
None of the display drivers now implement set_encoder_mode callback.
Stop calling it from the modeset init code.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/dsi/dsi.c | 2 --
drivers/gpu/drm/msm/dsi/dsi.h | 1 -
drivers/gpu/drm/msm/dsi/dsi_manager.c | 12
3 files changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 5201d7eb0490..77c8dba297d8 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -251,8 +251,6 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct
drm_device *dev,
goto fail;
}
- msm_dsi_manager_setup_encoder(msm_dsi->id);
-
priv->bridges[priv->num_bridges++] = msm_dsi->bridge;
priv->connectors[priv->num_connectors++] = msm_dsi->connector;
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 856a532850c0..e0c3c4409377 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -80,7 +80,6 @@ struct drm_connector *msm_dsi_manager_connector_init(u8 id);
struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id);
int msm_dsi_manager_cmd_xfer(int id, const struct mipi_dsi_msg *msg);
bool msm_dsi_manager_cmd_xfer_trigger(int id, u32 dma_base, u32 len);
-void msm_dsi_manager_setup_encoder(int id);
int msm_dsi_manager_register(struct msm_dsi *msm_dsi);
void msm_dsi_manager_unregister(struct msm_dsi *msm_dsi);
bool msm_dsi_manager_validate_current_config(u8 id);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 27d3b9ebf831..693078e68fd4 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -216,18 +216,6 @@ static int dsi_mgr_bridge_get_id(struct drm_bridge *bridge)
return dsi_bridge->id;
}
-void msm_dsi_manager_setup_encoder(int id)
-{
- struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
- struct msm_drm_private *priv = msm_dsi->dev->dev_private;
- struct msm_kms *kms = priv->kms;
- struct drm_encoder *encoder = msm_dsi_get_encoder(msm_dsi);
-
- if (encoder && kms->funcs->set_encoder_mode)
- kms->funcs->set_encoder_mode(kms, encoder,
-msm_dsi_is_cmd_mode(msm_dsi));
-}
-
static int msm_dsi_manager_panel_init(struct drm_connector *conn, u8 id)
{
struct msm_drm_private *priv = conn->dev->dev_private;
--
2.30.2
[PATCH v3 5/7] drm/msm/dp: stop calling set_encoder_mode callback
None of the display drivers now implement set_encoder_mode callback.
Stop calling it from the modeset init code.
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/dp/dp_display.c | 18 --
1 file changed, 18 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 051c1be1de7e..70b319a8fe83 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -102,8 +102,6 @@ struct dp_display_private {
struct dp_display_mode dp_mode;
struct msm_dp dp_display;
- bool encoder_mode_set;
-
/* wait for audio signaling */
struct completion audio_comp;
@@ -283,20 +281,6 @@ static void dp_display_send_hpd_event(struct msm_dp
*dp_display)
}
-static void dp_display_set_encoder_mode(struct dp_display_private *dp)
-{
- struct msm_drm_private *priv = dp->dp_display.drm_dev->dev_private;
- struct msm_kms *kms = priv->kms;
-
- if (!dp->encoder_mode_set && dp->dp_display.encoder &&
- kms->funcs->set_encoder_mode) {
- kms->funcs->set_encoder_mode(kms,
- dp->dp_display.encoder, false);
-
- dp->encoder_mode_set = true;
- }
-}
-
static int dp_display_send_hpd_notification(struct dp_display_private *dp,
bool hpd)
{
@@ -369,8 +353,6 @@ static void dp_display_host_init(struct dp_display_private
*dp, int reset)
if (dp->usbpd->orientation == ORIENTATION_CC2)
flip = true;
- dp_display_set_encoder_mode(dp);
-
dp_power_init(dp->power, flip);
dp_ctrl_host_init(dp->ctrl, flip, reset);
dp_aux_init(dp->aux);
--
2.30.2
[PATCH v3 1/7] drm/msm/dsi: rename dual DSI to bonded DSI
We are preparing to support two independent DSI hosts in the DSI/DPU
code. To remove possible confusion (as both configurations can be
referenced as dual DSI) let's rename old "dual DSI" (two DSI hosts
driving single device, with clocks being locked) to "bonded DSI".
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c | 2 +-
drivers/gpu/drm/msm/dsi/dsi.h | 8 +-
drivers/gpu/drm/msm/dsi/dsi_cfg.h | 2 +-
drivers/gpu/drm/msm/dsi/dsi_host.c | 34 +++
drivers/gpu/drm/msm/dsi/dsi_manager.c | 101 ++---
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 2 +-
drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 6 +-
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 2 +-
8 files changed, 78 insertions(+), 79 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c
index 81b0c7cf954e..1220f2b20e05 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_ctl.c
@@ -737,7 +737,7 @@ struct mdp5_ctl_manager *mdp5_ctlm_init(struct drm_device
*dev,
}
/*
-* In Dual DSI case, CTL0 and CTL1 are always assigned to two DSI
+* In bonded DSI case, CTL0 and CTL1 are always assigned to two DSI
* interfaces to support single FLUSH feature (Flush CTL0 and CTL1 when
* only write into CTL0's FLUSH register) to keep two DSI pipes in sync.
* Single FLUSH is supported from hw rev v3.0.
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 9b8e9b07eced..856a532850c0 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -109,7 +109,7 @@ int msm_dsi_host_enable(struct mipi_dsi_host *host);
int msm_dsi_host_disable(struct mipi_dsi_host *host);
int msm_dsi_host_power_on(struct mipi_dsi_host *host,
struct msm_dsi_phy_shared_timings *phy_shared_timings,
- bool is_dual_dsi);
+ bool is_bonded_dsi);
int msm_dsi_host_power_off(struct mipi_dsi_host *host);
int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
const struct drm_display_mode *mode);
@@ -123,7 +123,7 @@ int msm_dsi_host_set_src_pll(struct mipi_dsi_host *host,
void msm_dsi_host_reset_phy(struct mipi_dsi_host *host);
void msm_dsi_host_get_phy_clk_req(struct mipi_dsi_host *host,
struct msm_dsi_phy_clk_request *clk_req,
- bool is_dual_dsi);
+ bool is_bonded_dsi);
void msm_dsi_host_destroy(struct mipi_dsi_host *host);
int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
struct drm_device *dev);
@@ -145,8 +145,8 @@ int dsi_dma_base_get_6g(struct msm_dsi_host *msm_host,
uint64_t *iova);
int dsi_dma_base_get_v2(struct msm_dsi_host *msm_host, uint64_t *iova);
int dsi_clk_init_v2(struct msm_dsi_host *msm_host);
int dsi_clk_init_6g_v2(struct msm_dsi_host *msm_host);
-int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_dual_dsi);
-int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_dual_dsi);
+int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host, bool is_bonded_dsi);
+int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_bonded_dsi);
void msm_dsi_host_snapshot(struct msm_disp_state *disp_state, struct
mipi_dsi_host *host);
/* dsi phy */
struct msm_dsi_phy;
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
index ade9b609c7d9..2bce00d5a9fc 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
@@ -47,7 +47,7 @@ struct msm_dsi_host_cfg_ops {
void* (*tx_buf_get)(struct msm_dsi_host *msm_host);
void (*tx_buf_put)(struct msm_dsi_host *msm_host);
int (*dma_base_get)(struct msm_dsi_host *msm_host, uint64_t *iova);
- int (*calc_clk_rate)(struct msm_dsi_host *msm_host, bool is_dual_dsi);
+ int (*calc_clk_rate)(struct msm_dsi_host *msm_host, bool is_bonded_dsi);
};
struct msm_dsi_cfg_handler {
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index ed504fe5074f..706634d766ee 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -679,7 +679,7 @@ void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host)
clk_disable_unprepare(msm_host->byte_clk);
}
-static u32 dsi_get_pclk_rate(struct msm_dsi_host *msm_host, bool is_dual_dsi)
+static u32 dsi_get_pclk_rate(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
{
struct drm_display_mode *mode = msm_host->mode;
u32 pclk_rate;
@@ -687,22 +687,22 @@ static u32 dsi_get_pclk_rate(struct msm_dsi_host
*msm_host, bool is_dual_dsi)
pclk_rate = mode->clock * 1000;
/*
-* For dual DSI mode, the current DRM mode has the complete width of the
+* For bonded DSI mode, the current DRM mode has the complete width of
the
* pan
[PATCH v3 3/7] drm/msm/dpu: support setting up two independent DSI connectors
Move setting up encoders from set_encoder_mode to
_dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
allows us to support not only "single DSI" and "bonded DSI" but also "two
independent DSI" configurations. In future this would also help adding
support for multiple DP connectors.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 102 +---
1 file changed, 57 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1d3a4f395e74..3cd2011e18d4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -471,30 +471,68 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encoder = NULL;
+ struct msm_display_info info;
int i, rc = 0;
if (!(priv->dsi[0] || priv->dsi[1]))
return rc;
- /*TODO: Support two independent DSI connectors */
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
- if (IS_ERR(encoder)) {
- DPU_ERROR("encoder init failed for dsi display\n");
- return PTR_ERR(encoder);
- }
-
- priv->encoders[priv->num_encoders++] = encoder;
-
+ /*
+* We support following confiurations:
+* - Single DSI host (dsi0 or dsi1)
+* - Two independent DSI hosts
+* - Bonded DSI0 and DSI1 hosts
+*
+* TODO: Support swapping DSI0 and DSI1 in the bonded setup.
+*/
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+ int other = (i + 1) % 2;
+
if (!priv->dsi[i])
continue;
+ if (msm_dsi_is_bonded_dsi(priv->dsi[i]) &&
+ !msm_dsi_is_master_dsi(priv->dsi[i]))
+ continue;
+
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
+ if (IS_ERR(encoder)) {
+ DPU_ERROR("encoder init failed for dsi display\n");
+ return PTR_ERR(encoder);
+ }
+
+ priv->encoders[priv->num_encoders++] = encoder;
+
+ memset(&info, 0, sizeof(info));
+ info.intf_type = encoder->encoder_type;
+
rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
i, rc);
break;
}
+
+ info.h_tile_instance[info.num_of_h_tiles++] = i;
+ info.capabilities = msm_dsi_is_cmd_mode(priv->dsi[i]) ?
+ MSM_DISPLAY_CAP_CMD_MODE :
+ MSM_DISPLAY_CAP_VID_MODE;
+
+ if (msm_dsi_is_bonded_dsi(priv->dsi[i]) && priv->dsi[other]) {
+ rc = msm_dsi_modeset_init(priv->dsi[other], dev,
encoder);
+ if (rc) {
+ DPU_ERROR("modeset_init failed for dsi[%d], rc
= %d\n",
+ other, rc);
+ break;
+ }
+
+ info.h_tile_instance[info.num_of_h_tiles++] = other;
+ }
+
+ rc = dpu_encoder_setup(dev, encoder, &info);
+ if (rc)
+ DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
+ encoder->base.id, rc);
}
return rc;
@@ -505,6 +543,7 @@ static int _dpu_kms_initialize_displayport(struct
drm_device *dev,
struct dpu_kms *dpu_kms)
{
struct drm_encoder *encoder = NULL;
+ struct msm_display_info info;
int rc = 0;
if (!priv->dp)
@@ -516,6 +555,7 @@ static int _dpu_kms_initialize_displayport(struct
drm_device *dev,
return PTR_ERR(encoder);
}
+ memset(&info, 0, sizeof(info));
rc = msm_dp_modeset_init(priv->dp, dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
@@ -524,6 +564,14 @@ static int _dpu_kms_initialize_displayport(struct
drm_device *dev,
}
priv->encoders[priv->num_encoders++] = encoder;
+
+ info.num_of_h_tiles = 1;
+ info.capabilities = MSM_DISPLAY_CAP_VID_MODE;
+ info.intf_type = encoder->encoder_type;
+ rc = dpu_encoder_setup(dev, encoder, &info);
+ if (rc)
+ DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
+ encoder->base.id, rc);
return rc;
}
@@ -726,41 +774,6 @@ static void dpu_kms_destroy(struct msm_kms *kms)
msm_kms_destroy(&dpu_kms->base);
}
-static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
-struct drm_encoder *encoder,
-
[PATCH v3 2/7] drm/msm/dsi: add three helper functions
Add three helper functions to be used by display drivers for setting up
encoders.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi.c | 7 +++
drivers/gpu/drm/msm/dsi/dsi_manager.c | 19 +++
drivers/gpu/drm/msm/msm_drv.h | 17 +++--
3 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 75afc12a7b25..5201d7eb0490 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -13,6 +13,13 @@ struct drm_encoder *msm_dsi_get_encoder(struct msm_dsi
*msm_dsi)
return msm_dsi->encoder;
}
+bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
+{
+ unsigned long host_flags = msm_dsi_host_get_mode_flags(msm_dsi->host);
+
+ return !(host_flags & MIPI_DSI_MODE_VIDEO);
+}
+
static int dsi_get_phy(struct msm_dsi *msm_dsi)
{
struct platform_device *pdev = msm_dsi->pdev;
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index b20645ab279b..27d3b9ebf831 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -216,12 +216,6 @@ static int dsi_mgr_bridge_get_id(struct drm_bridge *bridge)
return dsi_bridge->id;
}
-static bool dsi_mgr_is_cmd_mode(struct msm_dsi *msm_dsi)
-{
- unsigned long host_flags = msm_dsi_host_get_mode_flags(msm_dsi->host);
- return !(host_flags & MIPI_DSI_MODE_VIDEO);
-}
-
void msm_dsi_manager_setup_encoder(int id)
{
struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
@@ -231,7 +225,7 @@ void msm_dsi_manager_setup_encoder(int id)
if (encoder && kms->funcs->set_encoder_mode)
kms->funcs->set_encoder_mode(kms, encoder,
-dsi_mgr_is_cmd_mode(msm_dsi));
+msm_dsi_is_cmd_mode(msm_dsi));
}
static int msm_dsi_manager_panel_init(struct drm_connector *conn, u8 id)
@@ -276,7 +270,7 @@ static int msm_dsi_manager_panel_init(struct drm_connector
*conn, u8 id)
if (other_dsi && other_dsi->panel && kms->funcs->set_split_display) {
kms->funcs->set_split_display(kms, master_dsi->encoder,
slave_dsi->encoder,
- dsi_mgr_is_cmd_mode(msm_dsi));
+ msm_dsi_is_cmd_mode(msm_dsi));
}
out:
@@ -839,3 +833,12 @@ void msm_dsi_manager_unregister(struct msm_dsi *msm_dsi)
msm_dsim->dsi[msm_dsi->id] = NULL;
}
+bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi)
+{
+ return IS_BONDED_DSI();
+}
+
+bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
+{
+ return IS_MASTER_DSI_LINK(msm_dsi->id);
+}
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 1a48a709ffb3..3d331acbc94a 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -350,7 +350,9 @@ void __exit msm_dsi_unregister(void);
int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev,
struct drm_encoder *encoder);
void msm_dsi_snapshot(struct msm_disp_state *disp_state, struct msm_dsi
*msm_dsi);
-
+bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi);
+bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi);
+bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi);
#else
static inline void __init msm_dsi_register(void)
{
@@ -367,7 +369,18 @@ static inline int msm_dsi_modeset_init(struct msm_dsi
*msm_dsi,
static inline void msm_dsi_snapshot(struct msm_disp_state *disp_state, struct
msm_dsi *msm_dsi)
{
}
-
+static inline bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
+{
+ return false;
+}
+static bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi)
+{
+ return false;
+}
+bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
+{
+ return false
+}
#endif
#ifdef CONFIG_DRM_MSM_DP
--
2.30.2
[PATCH v3 0/7] drm/msm/dpu: add support for independent DSI config
This patchseries adds support for independent DSI config to DPU1 display subdriver. Also drop one of msm_kms_funcs callbacks, made unnecessary now. Tested on RB5 (dpu, dsi). Previous iteration was tested by Alexey Minnekhanov. Changes since v2: - Removed Reviewed-By tags from changed patches (1, 2) - Changed more dual DSI mentions in the patch 1 - Added msm_dsi_is_master_dsi() helper - Rewrote dsi encoder setup function again basing on review by Abhinav Cahanges since v1: - Rewrote dsi encoder setup function by separating common code sequence and calling it either for the bonded interface or twice for each of the DSI hosts. Changes since RFC: - renamed dual DSI to bonded DSI as suggsted by Abhinav - added comments to _dpu_kms_initialize_dsi() regarding encoders usage
[PATCH 2/2] drm/i915/uapi: Add docs about immutability of engine sets and VMs
Signed-off-by: Jason Ekstrand
Cc: Daniel Vetter
---
include/uapi/drm/i915_drm.h | 39 ++---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e334a8b14ef2d..b6bfbda1258c8 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1676,15 +1676,25 @@ struct drm_i915_gem_context_param {
*/
#define I915_CONTEXT_PARAM_RECOVERABLE 0x8
- /*
-* The id of the associated virtual memory address space (ppGTT) of
-* this context. Can be retrieved and passed to another context
-* (on the same fd) for both to use the same ppGTT and so share
-* address layouts, and avoid reloading the page tables on context
-* switches between themselves.
-*
-* See DRM_I915_GEM_VM_CREATE and DRM_I915_GEM_VM_DESTROY.
-*/
+/*
+ * The id of the associated virtual memory address space (ppGTT) of
+ * this context. Can be retrieved and passed to another context
+ * (on the same fd) for both to use the same ppGTT and so share
+ * address layouts, and avoid reloading the page tables on context
+ * switches between themselves.
+ *
+ * The VM id should only be set via I915_CONTEXT_CREATE_EXT_SETPARAM.
+ *
+ * On GPUs with graphics version 12 and earlier, it may also be set via
+ * DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM. However, this is only for
+ * backwards compatibility with old userspace and should be considered
+ * deprecated. Starting with graphics version 13, it can only be set via
+ * I915_CONTEXT_CREATE_EXT_SETPARAM. When using setparam, it may only be
+ * set once for each context and, once the context has been used with any
+ * ioctl other than I915_CONTEXT_CREATE_EXT_SETPARAM, it may not be set.
+ *
+ * See DRM_I915_GEM_VM_CREATE and DRM_I915_GEM_VM_DESTROY.
+ */
#define I915_CONTEXT_PARAM_VM 0x9
/*
@@ -1700,8 +1710,15 @@ struct drm_i915_gem_context_param {
* to specify a gap in the array that can be filled in later, e.g. by a
* virtual engine used for load balancing.
*
- * Setting the number of engines bound to the context to 0, by passing a zero
- * sized argument, will revert back to default settings.
+ * The engine set should only be set via I915_CONTEXT_CREATE_EXT_SETPARAM.
+ *
+ * On GPUs with graphics version 12 and earlier, it may also be set via
+ * DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM. However, this is only for
+ * backwards compatibility with old userspace and should be considered
+ * deprecated. Starting with graphics version 13, it can only be set via
+ * I915_CONTEXT_CREATE_EXT_SETPARAM. When using setparam, it may only be
+ * set once for each context and, once the context has been used with any
+ * ioctl other than I915_CONTEXT_CREATE_EXT_SETPARAM, it may not be set.
*
* See struct i915_context_param_engines.
*
--
2.31.1
[PATCH 1/2] drm/i915: Don't allow setting I915_CONTEXT_PARAM_VM twice
Allowing setting it multiple times brings no real utility to the API, no
userspace relies on it, and it does make i915 a tiny bit more
complicated. Let's disallow it for now unless someone comes up with a
compelling reason to support it.
Signed-off-by: Jason Ekstrand
Cc: Daniel Vetter
---
drivers/gpu/drm/i915/gem/i915_gem_context.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 7d6f52d8a8012..5853737cc79f3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -319,7 +319,6 @@ static int set_proto_ctx_vm(struct drm_i915_file_private
*fpriv,
const struct drm_i915_gem_context_param *args)
{
struct drm_i915_private *i915 = fpriv->dev_priv;
- struct i915_address_space *vm;
if (args->size)
return -EINVAL;
@@ -327,17 +326,16 @@ static int set_proto_ctx_vm(struct drm_i915_file_private
*fpriv,
if (!HAS_FULL_PPGTT(i915))
return -ENODEV;
+ if (pc->vm)
+ return -EINVAL;
+
if (upper_32_bits(args->value))
return -ENOENT;
- vm = i915_gem_vm_lookup(fpriv, args->value);
- if (!vm)
+ pc->vm = i915_gem_vm_lookup(fpriv, args->value);
+ if (!pc->vm)
return -ENOENT;
- if (pc->vm)
- i915_vm_put(pc->vm);
- pc->vm = vm;
-
return 0;
}
--
2.31.1
[PATCH 0/2] drm/i915: Better document VM and engine set APIs
As per Daniel's request, this better documents the VM and engine set APIs including some discussion of the deprecation plan. Test-with: 20210710211923.784638-1-ja...@jlekstrand.net Cc: Daniel Vetter Jason Ekstrand (2): drm/i915: Don't allow setting I915_CONTEXT_PARAM_VM twice drm/i915/uapi: Add docs about immutability of engine sets and VMs drivers/gpu/drm/i915/gem/i915_gem_context.c | 12 +++ include/uapi/drm/i915_drm.h | 39 +++-- 2 files changed, 33 insertions(+), 18 deletions(-) -- 2.31.1
[PATCH] drm/msm: reduce usage of round_pixclk callback
The round_pixclk() callback returns different rate only on MDP4 in HDMI
(DTV) case. Stop using this callback in other cases to simplify
mode_valid callbacks.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 7 ---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 7 ---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 22 --
drivers/gpu/drm/msm/edp/edp_connector.c | 11 ---
drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 9 +
5 files changed, 5 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1d3a4f395e74..5bf66d885af3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -659,12 +659,6 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
return ret;
}
-static long dpu_kms_round_pixclk(struct msm_kms *kms, unsigned long rate,
- struct drm_encoder *encoder)
-{
- return rate;
-}
-
static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms)
{
int i;
@@ -861,7 +855,6 @@ static const struct msm_kms_funcs kms_funcs = {
.disable_vblank = dpu_kms_disable_vblank,
.check_modified_format = dpu_format_check_modified_format,
.get_format = dpu_get_msm_format,
- .round_pixclk= dpu_kms_round_pixclk,
.destroy = dpu_kms_destroy,
.set_encoder_mode = _dpu_kms_set_encoder_mode,
.snapshot= dpu_kms_mdp_snapshot,
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 15aed45022bc..40831f091c29 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -190,12 +190,6 @@ static void mdp5_complete_commit(struct msm_kms *kms,
unsigned crtc_mask)
mdp5_smp_complete_commit(mdp5_kms->smp, &global_state->smp);
}
-static long mdp5_round_pixclk(struct msm_kms *kms, unsigned long rate,
- struct drm_encoder *encoder)
-{
- return rate;
-}
-
static int mdp5_set_split_display(struct msm_kms *kms,
struct drm_encoder *encoder,
struct drm_encoder *slave_encoder,
@@ -285,7 +279,6 @@ static const struct mdp_kms_funcs kms_funcs = {
.wait_flush = mdp5_wait_flush,
.complete_commit = mdp5_complete_commit,
.get_format = mdp_get_format,
- .round_pixclk= mdp5_round_pixclk,
.set_split_display = mdp5_set_split_display,
.set_encoder_mode = mdp5_set_encoder_mode,
.destroy = mdp5_kms_destroy,
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 4ebfedc4a9ac..e9fa96ca9fa5 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -327,27 +327,6 @@ static int dsi_mgr_connector_get_modes(struct
drm_connector *connector)
return num;
}
-static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector
*connector,
- struct drm_display_mode *mode)
-{
- int id = dsi_mgr_connector_get_id(connector);
- struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
- struct drm_encoder *encoder = msm_dsi_get_encoder(msm_dsi);
- struct msm_drm_private *priv = connector->dev->dev_private;
- struct msm_kms *kms = priv->kms;
- long actual, requested;
-
- DBG("");
- requested = 1000 * mode->clock;
- actual = kms->funcs->round_pixclk(kms, requested, encoder);
-
- DBG("requested=%ld, actual=%ld", requested, actual);
- if (actual != requested)
- return MODE_CLOCK_RANGE;
-
- return MODE_OK;
-}
-
static struct drm_encoder *
dsi_mgr_connector_best_encoder(struct drm_connector *connector)
{
@@ -579,7 +558,6 @@ static const struct drm_connector_funcs
dsi_mgr_connector_funcs = {
static const struct drm_connector_helper_funcs dsi_mgr_conn_helper_funcs = {
.get_modes = dsi_mgr_connector_get_modes,
- .mode_valid = dsi_mgr_connector_mode_valid,
.best_encoder = dsi_mgr_connector_best_encoder,
};
diff --git a/drivers/gpu/drm/msm/edp/edp_connector.c
b/drivers/gpu/drm/msm/edp/edp_connector.c
index 73cb5fd97a5a..1dc6c7333c5a 100644
--- a/drivers/gpu/drm/msm/edp/edp_connector.c
+++ b/drivers/gpu/drm/msm/edp/edp_connector.c
@@ -60,17 +60,6 @@ static int edp_connector_mode_valid(struct drm_connector
*connector,
{
struct edp_connector *edp_connector = to_edp_connector(connector);
struct msm_edp *edp = edp_connector->edp;
- struct msm_drm_private *priv = connector->dev->dev_private;
- struct msm_kms *kms = priv->kms;
- long actual, requested;
-
- requested = 1000 * mode->clock;
- actual = kms->funcs->round_pixclk(kms,
- requested, edp_connector->edp->encoder);
-
- DBG("reque
Re: [PATCH 1/1] drm: bridge: Mark deprecated operations in drm_bridge_funcs
Hi Sam,
Thank you for the patch.
On Sat, Jul 10, 2021 at 10:42:40AM +0200, Sam Ravnborg wrote:
> drm_bridge_funcs includes several duplicated operations as atomic
> variants has been added over time.
s/has/have/
> New bridge drivers shall use the atomic variants - mark the deprecated
> operations to try to avoid usage in new bridge drivers.
>
> Signed-off-by: Sam Ravnborg
> Cc: Laurent Pinchart
> Cc: Andrzej Hajda
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
> ---
> include/drm/drm_bridge.h | 28 ++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 2195daa289d2..6805898d70f5 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -171,6 +171,11 @@ struct drm_bridge_funcs {
>* signals) feeding it is still running when this callback is called.
>*
>* The @disable callback is optional.
> + *
> + * NOTE:
> + *
> + * This is deprecated, do not use!
> + * New drivers shall use &drm_bridge_funcs.atomic_disable.
>*/
> void (*disable)(struct drm_bridge *bridge);
>
> @@ -190,6 +195,11 @@ struct drm_bridge_funcs {
>* called.
>*
>* The @post_disable callback is optional.
> + *
> + * NOTE:
> + *
> + * This is deprecated, do not use!
> + * New drivers shall use &drm_bridge_funcs.atomic_post_disable.
>*/
> void (*post_disable)(struct drm_bridge *bridge);
>
> @@ -213,11 +223,15 @@ struct drm_bridge_funcs {
>* For atomic drivers the adjusted_mode is the mode stored in
>* &drm_crtc_state.adjusted_mode.
>*
> - * NOTE:
> - *
>* If a need arises to store and access modes adjusted for other
>* locations than the connection between the CRTC and the first bridge,
>* the DRM framework will have to be extended with DRM bridge states.
The DRM framework *has* been extended with DRM bridge states :-) Should
this be dropped ?
Reviewed-by: Laurent Pinchart
> + *
> + * NOTE:
> + *
> + * This is deprecated, do not use!
> + * New drivers shall set their mode in &drm_bridge_funcs.atomic_enable
> + * operation.
>*/
> void (*mode_set)(struct drm_bridge *bridge,
>const struct drm_display_mode *mode,
> @@ -239,6 +253,11 @@ struct drm_bridge_funcs {
>* there is one) when this callback is called.
>*
>* The @pre_enable callback is optional.
> + *
> + * NOTE:
> + *
> + * This is deprecated, do not use!
> + * New drivers shall use &drm_bridge_funcs.atomic_pre_enable.
>*/
> void (*pre_enable)(struct drm_bridge *bridge);
>
> @@ -259,6 +278,11 @@ struct drm_bridge_funcs {
>* chain if there is one.
>*
>* The @enable callback is optional.
> + *
> + * NOTE:
> + *
> + * This is deprecated, do not use!
> + * New drivers shall use &drm_bridge_funcs.atomic_enable.
>*/
> void (*enable)(struct drm_bridge *bridge);
>
--
Regards,
Laurent Pinchart
Re: [PATCH] drm/shmobile: Convert to Linux IRQ interfaces
Hi Thomas,
Thank you for the patch.
On Tue, Jul 06, 2021 at 09:49:00AM +0200, Thomas Zimmermann wrote:
> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
> don't benefit from using it.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/shmobile/shmob_drm_drv.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> index 0a02b7092c04..032a2fff5efd 100644
> --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
> @@ -18,7 +18,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
>
> @@ -130,7 +129,6 @@ DEFINE_DRM_GEM_CMA_FOPS(shmob_drm_fops);
>
> static const struct drm_driver shmob_drm_driver = {
> .driver_features= DRIVER_GEM | DRIVER_MODESET,
> - .irq_handler= shmob_drm_irq,
> DRM_GEM_CMA_DRIVER_OPS,
> .fops = &shmob_drm_fops,
> .name = "shmob-drm",
> @@ -183,7 +181,7 @@ static int shmob_drm_remove(struct platform_device *pdev)
>
> drm_dev_unregister(ddev);
> drm_kms_helper_poll_fini(ddev);
> - drm_irq_uninstall(ddev);
> + free_irq(platform_get_irq(pdev, 0), ddev);
> drm_dev_put(ddev);
>
> return 0;
> @@ -258,7 +256,7 @@ static int shmob_drm_probe(struct platform_device *pdev)
> goto err_modeset_cleanup;
> }
>
> - ret = drm_irq_install(ddev, platform_get_irq(pdev, 0));
> + ret = request_irq(platform_get_irq(pdev, 0), shmob_drm_irq, 0,
> ddev->driver->name, ddev);
Could you store the irq number in a local variable, and wrap this line
at 80 columns ? You can then use the local variable in the free_irq()
call below. With this addressed,
Reviewed-by: Laurent Pinchart
> if (ret < 0) {
> dev_err(&pdev->dev, "failed to install IRQ handler\n");
> goto err_modeset_cleanup;
> @@ -275,7 +273,7 @@ static int shmob_drm_probe(struct platform_device *pdev)
> return 0;
>
> err_irq_uninstall:
> - drm_irq_uninstall(ddev);
> + free_irq(platform_get_irq(pdev, 0), ddev);
> err_modeset_cleanup:
> drm_kms_helper_poll_fini(ddev);
> err_free_drm_dev:
--
Regards,
Laurent Pinchart
Re: [Freedreno] [PATCH v2 3/7] drm/msm/dpu: support setting up two independent DSI connectors
On 2021-07-10 12:38, Dmitry Baryshkov wrote:
On 10/07/2021 03:30, abhin...@codeaurora.org wrote:
On 2021-07-09 16:50, Dmitry Baryshkov wrote:
Move setting up encoders from set_encoder_mode to
_dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
allows us to support not only "single DSI" and "bonded DSI" but also
"two
independent DSI" configurations. In future this would also help
adding
support for multiple DP connectors.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 130
++--
1 file changed, 79 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1d3a4f395e74..e14eb8f94cd7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -466,17 +466,16 @@ static void dpu_kms_wait_flush(struct msm_kms
*kms, unsigned crtc_mask)
dpu_kms_wait_for_commit_done(kms, crtc);
}
-static int _dpu_kms_initialize_dsi(struct drm_device *dev,
- struct msm_drm_private *priv,
- struct dpu_kms *dpu_kms)
+static int _dpu_kms_initialize_dsi_encoder(struct drm_device *dev,
+ struct msm_drm_private *priv,
+ struct dpu_kms *dpu_kms,
+ int dsi_id, int dsi_id1)
{
+ struct msm_dsi *dsi = priv->dsi[dsi_id];
struct drm_encoder *encoder = NULL;
- int i, rc = 0;
-
- if (!(priv->dsi[0] || priv->dsi[1]))
- return rc;
+ struct msm_display_info info;
+ int rc = 0;
- /*TODO: Support two independent DSI connectors */
encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
if (IS_ERR(encoder)) {
DPU_ERROR("encoder init failed for dsi display\n");
@@ -485,19 +484,74 @@ static int _dpu_kms_initialize_dsi(struct
drm_device *dev,
priv->encoders[priv->num_encoders++] = encoder;
- for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
- if (!priv->dsi[i])
- continue;
+ rc = msm_dsi_modeset_init(dsi, dev, encoder);
+ if (rc) {
+ DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
+ dsi_id, rc);
+ return rc;
+ }
+
+ memset(&info, 0, sizeof(info));
+ info.intf_type = encoder->encoder_type;
+ info.capabilities = msm_dsi_is_cmd_mode(dsi) ?
+ MSM_DISPLAY_CAP_CMD_MODE :
+ MSM_DISPLAY_CAP_VID_MODE;
+ info.h_tile_instance[info.num_of_h_tiles++] = dsi_id;
- rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
+ /* For the bonded DSI setup we have second DSI host */
+ if (dsi_id1 >= 0) {
+ struct msm_dsi *dsi1 = priv->dsi[dsi_id1];
+
+ rc = msm_dsi_modeset_init(dsi1, dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
- i, rc);
- break;
+ dsi_id1, rc);
+ return rc;
}
+
+ info.h_tile_instance[info.num_of_h_tiles++] = dsi_id1;
}
- return rc;
+ rc = dpu_encoder_setup(dev, encoder, &info);
+ if (rc) {
+ DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
+ encoder->base.id, rc);
+ return rc;
+ }
+
+ return 0;
+}
+
+static int _dpu_kms_initialize_dsi(struct drm_device *dev,
+ struct msm_drm_private *priv,
+ struct dpu_kms *dpu_kms)
+{
+ int i, rc = 0;
+
+ if (!(priv->dsi[0] || priv->dsi[1]))
+ return rc;
+
+ /*
+ * We support following confiurations:
+ * - Single DSI host (dsi0 or dsi1)
+ * - Two independent DSI hosts
+ * - Bonded DSI0 and DSI1 hosts
+ *
+ * TODO: Support swapping DSI0 and DSI1 in the bonded setup.
+ */
+ if (priv->dsi[0] && priv->dsi[1] &&
msm_dsi_is_bonded_dsi(priv->dsi[0]))
+ return _dpu_kms_initialize_dsi_encoder(dev, priv, dpu_kms,
0, 1);
+
+ for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+ if (!priv->dsi[i])
+ continue;
+
+ rc = _dpu_kms_initialize_dsi_encoder(dev, priv, dpu_kms, i,
-1);
+ if (rc)
+ return rc;
+ }
+
+ return 0;
}
Can we simplify this a bit like below?
static int _dpu_kms_initialize_dsi(struct drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms)
{
int i, rc = 0;
if (!(priv->dsi[0] || priv->dsi[1]))
return rc;
/*
* We support following confiurations:
* - Single DSI host (dsi0 or dsi1)
* - Two independent DSI hosts
* - Bonded DSI0 and DSI1 hosts
*
* TODO: Support swapping DSI0 and DSI1 in the bonded setup.
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
if (!priv->dsi[i])
continue;
rc = _dpu_kms_initialize_dsi(dev, priv, dpu_kms); // this API
does everything except encoder setup
if (rc)
return rc;
if (!is_bonded_dsi)
Re: [Freedreno] [PATCH v2 3/7] drm/msm/dpu: support setting up two independent DSI connectors
On 10/07/2021 03:30, abhin...@codeaurora.org wrote:
On 2021-07-09 16:50, Dmitry Baryshkov wrote:
Move setting up encoders from set_encoder_mode to
_dpu_kms_initialize_dsi() / _dpu_kms_initialize_displayport(). This
allows us to support not only "single DSI" and "bonded DSI" but also "two
independent DSI" configurations. In future this would also help adding
support for multiple DP connectors.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 130 ++--
1 file changed, 79 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1d3a4f395e74..e14eb8f94cd7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -466,17 +466,16 @@ static void dpu_kms_wait_flush(struct msm_kms
*kms, unsigned crtc_mask)
dpu_kms_wait_for_commit_done(kms, crtc);
}
-static int _dpu_kms_initialize_dsi(struct drm_device *dev,
- struct msm_drm_private *priv,
- struct dpu_kms *dpu_kms)
+static int _dpu_kms_initialize_dsi_encoder(struct drm_device *dev,
+ struct msm_drm_private *priv,
+ struct dpu_kms *dpu_kms,
+ int dsi_id, int dsi_id1)
{
+ struct msm_dsi *dsi = priv->dsi[dsi_id];
struct drm_encoder *encoder = NULL;
- int i, rc = 0;
-
- if (!(priv->dsi[0] || priv->dsi[1]))
- return rc;
+ struct msm_display_info info;
+ int rc = 0;
- /*TODO: Support two independent DSI connectors */
encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
if (IS_ERR(encoder)) {
DPU_ERROR("encoder init failed for dsi display\n");
@@ -485,19 +484,74 @@ static int _dpu_kms_initialize_dsi(struct
drm_device *dev,
priv->encoders[priv->num_encoders++] = encoder;
- for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
- if (!priv->dsi[i])
- continue;
+ rc = msm_dsi_modeset_init(dsi, dev, encoder);
+ if (rc) {
+ DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
+ dsi_id, rc);
+ return rc;
+ }
+
+ memset(&info, 0, sizeof(info));
+ info.intf_type = encoder->encoder_type;
+ info.capabilities = msm_dsi_is_cmd_mode(dsi) ?
+ MSM_DISPLAY_CAP_CMD_MODE :
+ MSM_DISPLAY_CAP_VID_MODE;
+ info.h_tile_instance[info.num_of_h_tiles++] = dsi_id;
- rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
+ /* For the bonded DSI setup we have second DSI host */
+ if (dsi_id1 >= 0) {
+ struct msm_dsi *dsi1 = priv->dsi[dsi_id1];
+
+ rc = msm_dsi_modeset_init(dsi1, dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
- i, rc);
- break;
+ dsi_id1, rc);
+ return rc;
}
+
+ info.h_tile_instance[info.num_of_h_tiles++] = dsi_id1;
}
- return rc;
+ rc = dpu_encoder_setup(dev, encoder, &info);
+ if (rc) {
+ DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
+ encoder->base.id, rc);
+ return rc;
+ }
+
+ return 0;
+}
+
+static int _dpu_kms_initialize_dsi(struct drm_device *dev,
+ struct msm_drm_private *priv,
+ struct dpu_kms *dpu_kms)
+{
+ int i, rc = 0;
+
+ if (!(priv->dsi[0] || priv->dsi[1]))
+ return rc;
+
+ /*
+ * We support following confiurations:
+ * - Single DSI host (dsi0 or dsi1)
+ * - Two independent DSI hosts
+ * - Bonded DSI0 and DSI1 hosts
+ *
+ * TODO: Support swapping DSI0 and DSI1 in the bonded setup.
+ */
+ if (priv->dsi[0] && priv->dsi[1] &&
msm_dsi_is_bonded_dsi(priv->dsi[0]))
+ return _dpu_kms_initialize_dsi_encoder(dev, priv, dpu_kms, 0,
1);
+
+ for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+ if (!priv->dsi[i])
+ continue;
+
+ rc = _dpu_kms_initialize_dsi_encoder(dev, priv, dpu_kms, i, -1);
+ if (rc)
+ return rc;
+ }
+
+ return 0;
}
Can we simplify this a bit like below?
static int _dpu_kms_initialize_dsi(struct drm_device *dev,
struct msm_drm_private *priv,
struct dpu_kms *dpu_kms)
{
int i, rc = 0;
if (!(priv->dsi[0] || priv->dsi[1]))
return rc;
/*
* We support following confiurations:
* - Single DSI host (dsi0 or dsi1)
* - Two independent DSI hosts
* - Bonded DSI0 and DSI1 hosts
*
* TODO: Support swapping DSI0 and DSI1 in the bonded setup.
for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
if (!priv->dsi[i])
continue;
rc = _dpu_kms_initialize_dsi(dev, priv, dpu_kms); // this API
does everything except encoder setup
if (rc)
return rc;
if (!is_bonded_dsi)
_dpu_kms_initialize_dsi_encoder(...);
Re: [Intel-gfx] [PATCH 43/47] drm/i915/guc: Hook GuC scheduling policies up
On Fri, Jun 25, 2021 at 12:10:46PM -0700, John Harrison wrote:
> On 6/24/2021 17:59, Matthew Brost wrote:
> > On Thu, Jun 24, 2021 at 12:05:12AM -0700, Matthew Brost wrote:
> > > From: John Harrison
> > >
> > > Use the official driver default scheduling policies for configuring
> > > the GuC scheduler rather than a bunch of hardcoded values.
> > >
> > > Signed-off-by: John Harrison
> > > Signed-off-by: Matthew Brost
> > > Cc: Jose Souza
> > > ---
> > > drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
> > > drivers/gpu/drm/i915/gt/uc/intel_guc.h| 2 +
> > > drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c| 44 ++-
> > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 11 +++--
> > > 4 files changed, 53 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > index 0ceffa2be7a7..37db857bb56c 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > @@ -455,6 +455,7 @@ struct intel_engine_cs {
> > > #define I915_ENGINE_IS_VIRTUAL BIT(5)
> > > #define I915_ENGINE_HAS_RELATIVE_MMIO BIT(6)
> > > #define I915_ENGINE_REQUIRES_CMD_PARSER BIT(7)
> > > +#define I915_ENGINE_WANT_FORCED_PREEMPTION BIT(8)
> > > unsigned int flags;
> > > /*
> > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > > b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > > index c38365cd5fab..905ecbc7dbe3 100644
> > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> > > @@ -270,6 +270,8 @@ int intel_guc_engine_failure_process_msg(struct
> > > intel_guc *guc,
> > > void intel_guc_find_hung_context(struct intel_engine_cs *engine);
> > > +int intel_guc_global_policies_update(struct intel_guc *guc);
> > > +
> > > void intel_guc_submission_reset_prepare(struct intel_guc *guc);
> > > void intel_guc_submission_reset(struct intel_guc *guc, bool stalled);
> > > void intel_guc_submission_reset_finish(struct intel_guc *guc);
> > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> > > b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> > > index d3e86ab7508f..2ad5fcd4e1b7 100644
> > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> > > @@ -77,14 +77,54 @@ static u32 guc_ads_blob_size(struct intel_guc *guc)
> > > guc_ads_private_data_size(guc);
> > > }
> > > -static void guc_policies_init(struct guc_policies *policies)
> > > +static void guc_policies_init(struct intel_guc *guc, struct guc_policies
> > > *policies)
> > > {
> > > + struct intel_gt *gt = guc_to_gt(guc);
> > > + struct drm_i915_private *i915 = gt->i915;
> > > +
> > > policies->dpc_promote_time =
> > > GLOBAL_POLICY_DEFAULT_DPC_PROMOTE_TIME_US;
> > > policies->max_num_work_items = GLOBAL_POLICY_MAX_NUM_WI;
> > > +
> > > policies->global_flags = 0;
> > > + if (i915->params.reset < 2)
> > > + policies->global_flags |= GLOBAL_POLICY_DISABLE_ENGINE_RESET;
> > > +
> > > policies->is_valid = 1;
> > > }
> > > +static int guc_action_policies_update(struct intel_guc *guc, u32
> > > policy_offset)
> > > +{
> > > + u32 action[] = {
> > > + INTEL_GUC_ACTION_GLOBAL_SCHED_POLICY_CHANGE,
> > > + policy_offset
> > > + };
> > > +
> > > + return intel_guc_send(guc, action, ARRAY_SIZE(action));
> > > +}
> > > +
> > > +int intel_guc_global_policies_update(struct intel_guc *guc)
> > > +{
> > > + struct __guc_ads_blob *blob = guc->ads_blob;
> > > + struct intel_gt *gt = guc_to_gt(guc);
> > > + intel_wakeref_t wakeref;
> > > + int ret;
> > > +
> > > + if (!blob)
> > > + return -ENOTSUPP;
> > > +
> > > + GEM_BUG_ON(!blob->ads.scheduler_policies);
> > > +
> > > + guc_policies_init(guc, &blob->policies);
> > > +
> > > + if (!intel_guc_is_ready(guc))
> > > + return 0;
> > > +
> > > + with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
> > > + ret = guc_action_policies_update(guc,
> > > blob->ads.scheduler_policies);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > static void guc_mapping_table_init(struct intel_gt *gt,
> > > struct guc_gt_system_info
> > > *system_info)
> > > {
> > > @@ -281,7 +321,7 @@ static void __guc_ads_init(struct intel_guc *guc)
> > > u8 engine_class, guc_class;
> > > /* GuC scheduling policies */
> > > - guc_policies_init(&blob->policies);
> > > + guc_policies_init(guc, &blob->policies);
> > > /*
> > >* GuC expects a per-engine-class context image and size
> > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > > b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > > index 6188189314d5..a427336ce916 100644
> > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submi
Re: [Intel-gfx] [PATCH 16/16] drm/i915/guc/rc: Setup and enable GUCRC feature
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> This feature hands over the control of HW RC6 to the GUC.
> GUC decides when to put HW into RC6 based on it's internal
> busyness algorithms.
>
> GUCRC needs GUC submission to be enabled, and only
> supported on Gen12+ for now.
>
> When GUCRC is enabled, do not set HW RC6. Use a H2G message
> to tell guc to enable GUCRC. When disabling RC6, tell guc to
s/GUC/GuC
s/guc/GuC
> revert RC6 control back to KMD.
>
> Signed-off-by: Vinay Belgaumkar
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/gt/intel_rc6.c | 22 --
> .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 6 ++
> drivers/gpu/drm/i915/gt/uc/intel_guc.c| 1 +
> drivers/gpu/drm/i915/gt/uc/intel_guc.h| 2 +
> drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 79 +++
> drivers/gpu/drm/i915/gt/uc/intel_guc_rc.h | 32
> drivers/gpu/drm/i915/gt/uc/intel_uc.h | 2 +
> 8 files changed, 140 insertions(+), 5 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c
> create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_rc.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index d8eac4468df9..3fc17f20d88e 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -186,6 +186,7 @@ i915-y += gt/uc/intel_uc.o \
> gt/uc/intel_guc_fw.o \
> gt/uc/intel_guc_log.o \
> gt/uc/intel_guc_log_debugfs.o \
> + gt/uc/intel_guc_rc.o \
> gt/uc/intel_guc_slpc.o \
> gt/uc/intel_guc_submission.o \
> gt/uc/intel_huc.o \
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c
> b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index 259d7eb4e165..299fcf10b04b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -98,11 +98,19 @@ static void gen11_rc6_enable(struct intel_rc6 *rc6)
> set(uncore, GEN9_MEDIA_PG_IDLE_HYSTERESIS, 60);
> set(uncore, GEN9_RENDER_PG_IDLE_HYSTERESIS, 60);
>
> - /* 3a: Enable RC6 */
> - rc6->ctl_enable =
> - GEN6_RC_CTL_HW_ENABLE |
> - GEN6_RC_CTL_RC6_ENABLE |
> - GEN6_RC_CTL_EI_MODE(1);
> + /* 3a: Enable RC6
> + *
> + * With GUCRC, we do not enable bit 31 of RC_CTL,
> + * thus allowing GuC to control RC6 entry/exit fully instead.
> + * We will not set the HW ENABLE and EI bits
> + */
> + if (!intel_guc_rc_enable(>->uc.guc))
> + rc6->ctl_enable = GEN6_RC_CTL_RC6_ENABLE;
> + else
> + rc6->ctl_enable =
> + GEN6_RC_CTL_HW_ENABLE |
> + GEN6_RC_CTL_RC6_ENABLE |
> + GEN6_RC_CTL_EI_MODE(1);
>
> pg_enable =
> GEN9_RENDER_PG_ENABLE |
> @@ -513,6 +521,10 @@ static void __intel_rc6_disable(struct intel_rc6 *rc6)
> {
> struct drm_i915_private *i915 = rc6_to_i915(rc6);
> struct intel_uncore *uncore = rc6_to_uncore(rc6);
> + struct intel_gt *gt = rc6_to_gt(rc6);
> +
> + /* Take control of RC6 back from GuC */
> + intel_guc_rc_disable(>->uc.guc);
>
> intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
> if (GRAPHICS_VER(i915) >= 9)
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> index 596cf4b818e5..2ddb9cdc0a59 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
> @@ -136,6 +136,7 @@ enum intel_guc_action {
> INTEL_GUC_ACTION_CONTEXT_RESET_NOTIFICATION = 0x1008,
> INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009,
> INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003,
> + INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004,
> INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
> INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502,
> INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503,
> @@ -146,6 +147,11 @@ enum intel_guc_action {
> INTEL_GUC_ACTION_LIMIT
> };
>
> +enum intel_guc_rc_options {
> + INTEL_GUCRC_HOST_CONTROL,
> + INTEL_GUCRC_FIRMWARE_CONTROL,
> +};
> +
> enum intel_guc_preempt_options {
> INTEL_GUC_PREEMPT_OPTION_DROP_WORK_Q = 0x4,
> INTEL_GUC_PREEMPT_OPTION_DROP_SUBMIT_Q = 0x8,
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 82863a9bc8e8..0d55b24f7c67 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -158,6 +158,7 @@ void intel_guc_init_early(struct intel_guc *guc)
> intel_guc_log_init_early(&guc->log);
> intel_guc_submission_init_early(guc);
> intel_guc_slpc_init_early(guc);
> + intel_guc_rc_init_early(guc);
>
> mutex_init(&guc->send_mutex);
> spin_lock_init(&guc->irq_lock);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index 0dbbd9cf553f
Re: [PATCH 15/16] drm/i915/guc/slpc: slpc selftest
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Tests that exercise the slpc get/set frequency interfaces.
>
> Clamp_max will set max frequency to multiple levels and check
> that slpc requests frequency lower than or equal to it.
>
> Clamp_min will set min frequency to different levels and check
> if slpc requests are higher or equal to those levels.
2x s/slpc/SLPC
>
> Signed-off-by: Vinay Belgaumkar
> ---
> drivers/gpu/drm/i915/gt/intel_rps.c | 1 +
> drivers/gpu/drm/i915/gt/selftest_slpc.c | 333 ++
> drivers/gpu/drm/i915/gt/selftest_slpc.h | 12 +
> .../drm/i915/selftests/i915_live_selftests.h | 1 +
> 4 files changed, 347 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/gt/selftest_slpc.c
> create mode 100644 drivers/gpu/drm/i915/gt/selftest_slpc.h
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
> b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 88ffc5d90730..16ac2e840881 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -2288,4 +2288,5 @@ EXPORT_SYMBOL_GPL(i915_gpu_turbo_disable);
>
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> #include "selftest_rps.c"
> +#include "selftest_slpc.c"
> #endif
> diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c
> b/drivers/gpu/drm/i915/gt/selftest_slpc.c
> new file mode 100644
> index ..f440c1cb2afa
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
> @@ -0,0 +1,333 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2020 Intel Corporation
2021
> + */
> +#include "selftest_slpc.h"
> +#include "selftest_rps.h"
> +
> +#include
> +#include
system headers should go first
> +
> +#include "intel_engine_heartbeat.h"
> +#include "intel_engine_pm.h"
> +#include "intel_gpu_commands.h"
> +#include "intel_gt_clock_utils.h"
> +#include "intel_gt_pm.h"
> +#include "intel_rc6.h"
> +#include "selftest_engine_heartbeat.h"
> +#include "intel_rps.h"
> +#include "selftests/igt_flush_test.h"
> +#include "selftests/igt_spinner.h"
wrong order ?
> +
> +#define NUM_STEPS 5
> +#define H2G_DELAY 5
> +#define delay_for_h2g() usleep_range(H2G_DELAY, H2G_DELAY + 1)
> +
> +static int set_min_freq(struct intel_guc_slpc *slpc, int freq)
> +{
> + int ret;
add empty line
> + ret = intel_guc_slpc_set_min_freq(slpc, freq);
> + if (ret) {
> + pr_err("Could not set min frequency to [%d]\n", freq);
> + return ret;
> + } else {
> + /* Delay to ensure h2g completes */
> + delay_for_h2g();
> + }
> +
> + return ret;
> +}
> +
> +static int set_max_freq(struct intel_guc_slpc *slpc, int freq)
> +{
> + int ret;
add empty line
> + ret = intel_guc_slpc_set_max_freq(slpc, freq);
> + if (ret) {
> + pr_err("Could not set maximum frequency [%d]\n",
> + freq);
> + return ret;
> + } else {
> + /* Delay to ensure h2g completes */
> + delay_for_h2g();
> + }
> +
> + return ret;
> +}
> +
> +int live_slpc_clamp_min(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct intel_gt *gt = &i915->gt;
> + struct intel_guc_slpc *slpc;
> + struct intel_rps *rps;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> + struct igt_spinner spin;
> + int err = 0;
usually "err" is last decl
> + u32 slpc_min_freq, slpc_max_freq;
> +
> +
too many empty lines
> + slpc = >->uc.guc.slpc;
> + rps = >->rps;
could be initialized in decl above
> +
> + if (!intel_uc_uses_guc_slpc(>->uc))
> + return 0;
> +
> + if (igt_spinner_init(&spin, gt))
> + return -ENOMEM;
> +
> + if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
> + pr_err("Could not get SLPC max freq");
> + return -EIO;
> + }
> +
> + if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
> + pr_err("Could not get SLPC min freq");
> + return -EIO;
> + }
> +
> + if (slpc_min_freq == slpc_max_freq) {
> + pr_err("Min/Max are fused to the same value");
> + return -EINVAL;
> + }
3x missing \n
> +
> + intel_gt_pm_wait_for_idle(gt);
> + intel_gt_pm_get(gt);
> + for_each_engine(engine, gt, id) {
> + struct i915_request *rq;
> + u32 step, min_freq, req_freq;
> + u32 act_freq, max_act_freq;
> +
> + if (!intel_engine_can_store_dword(engine))
> + continue;
> +
> + /* Go from min to max in 5 steps */
> + step = (slpc_max_freq - slpc_min_freq)/NUM_STEPS;
add spaces ") / NUM"
> + max_act_freq = slpc_min_freq;
> + for (min_freq = slpc_min_freq; min_freq < slpc_max_freq;
> min_freq+=step)
add spaces " += "
> + {
> + err = set_min_freq(slpc, min_freq);
> + if (err)
> +
Re: [PATCH 14/16] drm/i915/guc/slpc: Sysfs hooks for slpc
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Update the get/set min/max freq hooks to work for
> slpc case as well. Consolidate helpers for requested/min/max
> frequency get/set to intel_rps where the proper action can
> be taken depending on whether slpc is enabled.
2x s/slpc/SLPC
>
> Signed-off-by: Vinay Belgaumkar
> Signed-off-by: Tvrtko Ursulin
> Signed-off-by: Sujaritha Sundaresan
> ---
> drivers/gpu/drm/i915/gt/intel_rps.c | 135
> drivers/gpu/drm/i915/gt/intel_rps.h | 5 ++
> drivers/gpu/drm/i915/i915_pmu.c | 2 +-
> drivers/gpu/drm/i915/i915_reg.h | 2 +
> drivers/gpu/drm/i915/i915_sysfs.c | 71 +++
> 5 files changed, 154 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
> b/drivers/gpu/drm/i915/gt/intel_rps.c
> index e858eeb2c59d..88ffc5d90730 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -37,6 +37,12 @@ static struct intel_uncore *rps_to_uncore(struct intel_rps
> *rps)
> return rps_to_gt(rps)->uncore;
> }
>
> +static struct intel_guc_slpc *rps_to_slpc(struct intel_rps *rps)
> +{
> + struct intel_gt *gt = rps_to_gt(rps);
> + return >->uc.guc.slpc;
either add empty line between decl/code or make it one-liner
> +}
> +
> static bool rps_uses_slpc(struct intel_rps *rps)
> {
> struct intel_gt *gt = rps_to_gt(rps);
> @@ -1960,6 +1966,135 @@ u32 intel_rps_read_actual_frequency(struct intel_rps
> *rps)
> return freq;
> }
>
> +u32 intel_rps_read_punit_req(struct intel_rps *rps)
> +{
> + struct intel_uncore *uncore = rps_to_uncore(rps);
> +
drop empty line
> + u32 pureq = intel_uncore_read(uncore, GEN6_RPNSWREQ);
> +
> + return pureq;
> +}
> +
> +u32 intel_rps_get_req(struct intel_rps *rps, u32 pureq)
> +{
> + u32 req = pureq >> GEN9_SW_REQ_UNSLICE_RATIO_SHIFT;
> +
> + return req;
> +}
> +
> +u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps)
> +{
> + u32 freq = intel_rps_get_req(rps, intel_rps_read_punit_req(rps));
> +
> + return intel_gpu_freq(rps, freq);
> +}
> +
> +u32 intel_rps_get_requested_frequency(struct intel_rps *rps)
> +{
> + if (rps_uses_slpc(rps))
> + return intel_rps_read_punit_req_frequency(rps);
> + else
> + return intel_gpu_freq(rps, rps->cur_freq);
> +}
> +
> +u32 intel_rps_get_max_frequency(struct intel_rps *rps)
> +{
> + struct intel_guc_slpc *slpc = rps_to_slpc(rps);
> +
> + if (rps_uses_slpc(rps))
> + return slpc->max_freq_softlimit;
> + else
> + return intel_gpu_freq(rps, rps->max_freq_softlimit);
> +}
> +
> +int intel_rps_set_max_frequency(struct intel_rps *rps, u32 val)
> +{
> + struct intel_guc_slpc *slpc = rps_to_slpc(rps);
> + int ret;
> +
> + if (rps_uses_slpc(rps))
> + return intel_guc_slpc_set_max_freq(slpc, val);
> +
> + mutex_lock(&rps->lock);
> +
> + val = intel_freq_opcode(rps, val);
> + if (val < rps->min_freq ||
> + val > rps->max_freq ||
> + val < rps->min_freq_softlimit) {
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + if (val > rps->rp0_freq)
> + DRM_DEBUG("User requested overclocking to %d\n",
use drm_dbg
Michal
> + intel_gpu_freq(rps, val));
> +
> + rps->max_freq_softlimit = val;
> +
> + val = clamp_t(int, rps->cur_freq,
> + rps->min_freq_softlimit,
> + rps->max_freq_softlimit);
> +
> + /*
> + * We still need *_set_rps to process the new max_delay and
> + * update the interrupt limits and PMINTRMSK even though
> + * frequency request may be unchanged.
> + */
> + intel_rps_set(rps, val);
> +
> +unlock:
> + mutex_unlock(&rps->lock);
> +
> + return ret;
> +}
> +
> +u32 intel_rps_get_min_frequency(struct intel_rps *rps)
> +{
> + struct intel_guc_slpc *slpc = rps_to_slpc(rps);
> +
> + if (rps_uses_slpc(rps))
> + return slpc->min_freq_softlimit;
> + else
> + return intel_gpu_freq(rps, rps->min_freq_softlimit);
> +}
> +
> +int intel_rps_set_min_frequency(struct intel_rps *rps, u32 val)
> +{
> + struct intel_guc_slpc *slpc = rps_to_slpc(rps);
> + int ret;
> +
> + if (rps_uses_slpc(rps))
> + return intel_guc_slpc_set_min_freq(slpc, val);
> +
> + mutex_lock(&rps->lock);
> +
> + val = intel_freq_opcode(rps, val);
> + if (val < rps->min_freq ||
> + val > rps->max_freq ||
> + val > rps->max_freq_softlimit) {
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + rps->min_freq_softlimit = val;
> +
> + val = clamp_t(int, rps->cur_freq,
> + rps->min_freq_softlimit,
> + rps->max_freq_softlimit);
> +
> + /*
> + * We still need *_set_rps to process the new min_delay and
> + * update the interrupt limits and PMINTRMSK
Re: [Intel-gfx] [PATCH 12/16] drm/i915/guc/slpc: Cache platform frequency limits for slpc
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Cache rp0, rp1 and rpn platform limits into slpc structure
> for range checking while setting min/max frequencies.
>
> Also add "soft" limits which keep track of frequency changes
> made from userland. These are initially set to platform min
> and max.
>
> Signed-off-by: Vinay Belgaumkar
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 41 +
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index d32274cd1db7..6e978f27b7a6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -86,6 +86,9 @@ static int slpc_shared_data_init(struct intel_guc_slpc
> *slpc)
> return err;
> }
>
> + slpc->max_freq_softlimit = 0;
> + slpc->min_freq_softlimit = 0;
as mentioned earlier, now it is time to introduce these fields in .h
> +
> return err;
> }
>
> @@ -384,6 +387,29 @@ void intel_guc_pm_intrmsk_enable(struct intel_gt *gt)
> GEN6_PMINTRMSK, pm_intrmsk_mbz, 0);
> }
>
> +static int intel_guc_slpc_set_softlimits(struct intel_guc_slpc *slpc)
> +{
> + int ret = 0;
> +
> + /* Softlimits are initially equivalent to platform limits
> + * unless they have deviated from defaults, in which case,
> + * we retain the values and set min/max accordingly.
> + */
> + if (!slpc->max_freq_softlimit)
> + slpc->max_freq_softlimit = slpc->rp0_freq;
> + else if (slpc->max_freq_softlimit != slpc->rp0_freq)
> + ret = intel_guc_slpc_set_max_freq(slpc,
> + slpc->max_freq_softlimit);
> +
> + if (!slpc->min_freq_softlimit)
> + slpc->min_freq_softlimit = slpc->min_freq;
> + else if (slpc->min_freq_softlimit != slpc->min_freq)
> + ret = intel_guc_slpc_set_min_freq(slpc,
> + slpc->min_freq_softlimit);
> +
> + return ret;
> +}
> +
> /*
> * intel_guc_slpc_enable() - Start SLPC
> * @slpc: pointer to intel_guc_slpc.
> @@ -402,6 +428,7 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
> struct drm_i915_private *i915 = slpc_to_i915(slpc);
> struct slpc_shared_data *data;
> int ret;
> + u32 rp_state_cap;
move up to keep "ret" last
>
> GEM_BUG_ON(!slpc->vma);
>
> @@ -445,6 +472,20 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>
> DIV_ROUND_CLOSEST(data->task_state_data.max_unslice_freq *
> GT_FREQUENCY_MULTIPLIER, GEN9_FREQ_SCALER));
>
> + rp_state_cap = intel_uncore_read(i915->gt.uncore, GEN6_RP_STATE_CAP);
> +
> + slpc->rp0_freq = ((rp_state_cap >> 0) & 0xff) * GT_FREQUENCY_MULTIPLIER;
> + slpc->min_freq = ((rp_state_cap >> 16) & 0xff) *
> GT_FREQUENCY_MULTIPLIER;
> + slpc->rp1_freq = ((rp_state_cap >> 8) & 0xff) * GT_FREQUENCY_MULTIPLIER;
we should have definitions for these bits and then we should be able to
use REG_FIELD_GET
> +
> + if (intel_guc_slpc_set_softlimits(slpc))
> + drm_err(&i915->drm, "Unable to set softlimits");
missing \n
maybe we can also print error ?
> +
> + drm_info(&i915->drm,
> + "Platform fused frequency values - min: %u Mhz, max: %u Mhz",
missing \n
double space before 'min'
Michal
> + slpc->min_freq,
> + slpc->rp0_freq);
> +
> return 0;
> }
>
>
Re: [PATCH 10/16] drm/i915/guc/slpc: Add debugfs for slpc info
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> This prints out relevant SLPC info from the SLPC shared structure.
>
> We will send a h2g message which forces SLPC to update the
> shared data structure with latest information before reading it.
>
> Signed-off-by: Vinay Belgaumkar
> Signed-off-by: Sundaresan Sujaritha
> ---
> .../gpu/drm/i915/gt/uc/intel_guc_debugfs.c| 16 ++
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 53 +++
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 3 ++
> 3 files changed, 72 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c
> index 9a03ff56e654..bef749e54601 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_debugfs.c
> @@ -12,6 +12,7 @@
> #include "gt/uc/intel_guc_ct.h"
> #include "gt/uc/intel_guc_ads.h"
> #include "gt/uc/intel_guc_submission.h"
> +#include "gt/uc/intel_guc_slpc.h"
>
> static int guc_info_show(struct seq_file *m, void *data)
> {
> @@ -50,11 +51,26 @@ static int guc_registered_contexts_show(struct seq_file
> *m, void *data)
> }
> DEFINE_GT_DEBUGFS_ATTRIBUTE(guc_registered_contexts);
>
> +static int guc_slpc_info_show(struct seq_file *m, void *unused)
> +{
> + struct intel_guc *guc = m->private;
> + struct intel_guc_slpc *slpc = &guc->slpc;
> + struct drm_printer p = drm_seq_file_printer(m);
> +
> + if (!intel_guc_slpc_is_used(guc))
> + return -ENODEV;
> +
> + return intel_guc_slpc_info(slpc, &p);
> +}
> +
other entries don't have empty line here
> +DEFINE_GT_DEBUGFS_ATTRIBUTE(guc_slpc_info);
> +
> void intel_guc_debugfs_register(struct intel_guc *guc, struct dentry *root)
> {
> static const struct debugfs_gt_file files[] = {
> { "guc_info", &guc_info_fops, NULL },
> { "guc_registered_contexts", &guc_registered_contexts_fops,
> NULL },
> + { "guc_slpc_info", &guc_slpc_info_fops, NULL},
IIRC last field is "eval" where maybe you could add your own to check if
intel_guc_slpc_is_used() to avoid exposing this info if N/A
> };
>
> if (!intel_guc_is_supported(guc))
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index 98a283d31734..d179ba14ece6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -432,6 +432,59 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
> return 0;
> }
>
> +int intel_guc_slpc_info(struct intel_guc_slpc *slpc, struct drm_printer *p)
> +{
> + struct drm_i915_private *i915 = guc_to_gt(slpc_to_guc(slpc))->i915;
> + struct slpc_shared_data *data;
> + struct slpc_platform_info *platform_info;
> + struct slpc_task_state_data *task_state_data;
> + intel_wakeref_t wakeref;
> + int ret = 0;
> +
> + wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +
> + if (slpc_read_task_state(slpc)) {
> + ret = -EIO;
> + goto done;
> + }
> +
> + GEM_BUG_ON(!slpc->vma);
> +
> + drm_clflush_virt_range(slpc->vaddr, sizeof(struct slpc_shared_data));
likely will go away if integrated into slpc_read_task_state
> + data = slpc->vaddr;
> +
> + platform_info = &data->platform_info;
is this used ?
> + task_state_data = &data->task_state_data;
as it looks that you treat these sections separately, then maybe it
would be cleaner to have:
static void print_global_data(*global_data, *p) {}
static void print_platform_info(*platform_info, *p) {}
static void print_task_state_data(*task_state_data, *p) {}
> +
> + drm_printf(p, "SLPC state: %s\n",
> slpc_state_stringify(data->global_state));
> + drm_printf(p, "\tgtperf task active: %d\n",
> + task_state_data->gtperf_task_active);
> + drm_printf(p, "\tdcc task active: %d\n",
> + task_state_data->dcc_task_active);
> + drm_printf(p, "\tin dcc: %d\n",
> + task_state_data->in_dcc);
> + drm_printf(p, "\tfreq switch active: %d\n",
> + task_state_data->freq_switch_active);
> + drm_printf(p, "\tibc enabled: %d\n",
> + task_state_data->ibc_enabled);
> + drm_printf(p, "\tibc active: %d\n",
> + task_state_data->ibc_active);
> + drm_printf(p, "\tpg1 enabled: %s\n",
> + yesno(task_state_data->pg1_enabled));
> + drm_printf(p, "\tpg1 active: %s\n",
> + yesno(task_state_data->pg1_active));
> + drm_printf(p, "\tmax freq: %dMHz\n",
> +
> DIV_ROUND_CLOSEST(data->task_state_data.max_unslice_freq *
> + GT_FREQUENCY_MULTIPLIER, GEN9_FREQ_SCALER));
> + drm_printf(p, "\tmin freq: %dMHz\n",
> +
> DIV_ROUND_CLOSEST(dat
Re: [PATCH 09/16] drm/i915/guc/slpc: Add get max/min freq hooks
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Add helpers to read the min/max frequency being used
> by SLPC. This is done by send a h2g command which forces
s/h2g/H2G
> SLPC to update the shared data struct which can then be
> read.
>
> Signed-off-by: Vinay Belgaumkar
> Signed-off-by: Sundaresan Sujaritha
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 58 +
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 2 +
> 2 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index 19cb26479942..98a283d31734 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -278,6 +278,35 @@ int intel_guc_slpc_set_max_freq(struct intel_guc_slpc
> *slpc, u32 val)
> return ret;
> }
>
> +int intel_guc_slpc_get_max_freq(struct intel_guc_slpc *slpc, u32 *val)
> +{
> + struct slpc_shared_data *data;
> + intel_wakeref_t wakeref;
> + struct drm_i915_private *i915 = guc_to_gt(slpc_to_guc(slpc))->i915;
> + int ret = 0;
> +
> + wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +
> + /* Force GuC to update task data */
> + if (slpc_read_task_state(slpc)) {
> + DRM_ERROR("Unable to update task data");
use drm_err
missing \n
maybe this message could be moved to slpc_read_task_state ?
> + ret = -EIO;
> + goto done;
> + }
> +
> + GEM_BUG_ON(!slpc->vma);
> +
> + drm_clflush_virt_range(slpc->vaddr, sizeof(struct slpc_shared_data));
maybe this can also be part of slpc_read_task_state ?
> + data = slpc->vaddr;
> +
> + *val = DIV_ROUND_CLOSEST(data->task_state_data.max_unslice_freq *
> + GT_FREQUENCY_MULTIPLIER, GEN9_FREQ_SCALER);
> +
> +done:
> + intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> + return ret;
> +}
> +
> /**
> * intel_guc_slpc_min_freq_set() - Set min frequency limit for SLPC.
> * @slpc: pointer to intel_guc_slpc.
> @@ -312,6 +341,35 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
> *slpc, u32 val)
> return ret;
> }
>
> +int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val)
missing kernel-doc (above intel_guc_slpc_min_freq_set has one)
> +{
> + struct slpc_shared_data *data;
> + intel_wakeref_t wakeref;
> + struct drm_i915_private *i915 = guc_to_gt(slpc_to_guc(slpc))->i915;
> + int ret = 0;
> +
> + wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +
> + /* Force GuC to update task data */
> + if (slpc_read_task_state(slpc)) {
> + DRM_ERROR("Unable to update task data");
see above
> + ret = -EIO;
> + goto done;
> + }
> +
> + GEM_BUG_ON(!slpc->vma);
> +
> + drm_clflush_virt_range(slpc->vaddr, sizeof(struct slpc_shared_data));
see above
Michal
> + data = slpc->vaddr;
> +
> + *val = DIV_ROUND_CLOSEST(data->task_state_data.min_unslice_freq *
> + GT_FREQUENCY_MULTIPLIER, GEN9_FREQ_SCALER);
> +
> +done:
> + intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> + return ret;
> +}
> +
> /*
> * intel_guc_slpc_enable() - Start SLPC
> * @slpc: pointer to intel_guc_slpc.
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> index a473e1ea7c10..2cb830cdacb5 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> @@ -36,5 +36,7 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc);
> void intel_guc_slpc_fini(struct intel_guc_slpc *slpc);
> int intel_guc_slpc_set_max_freq(struct intel_guc_slpc *slpc, u32 val);
> int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val);
> +int intel_guc_slpc_get_max_freq(struct intel_guc_slpc *slpc, u32 *val);
> +int intel_guc_slpc_get_min_freq(struct intel_guc_slpc *slpc, u32 *val);
>
> #endif
>
Re: [PATCH 08/16] drm/i915/guc/slpc: Add methods to set min/max frequency
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Add param set h2g helpers to set the min and max frequencies
> for use by SLPC.
>
> Signed-off-by: Sundaresan Sujaritha
> Signed-off-by: Vinay Belgaumkar
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 94 +
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 2 +
> 2 files changed, 96 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index e579408d1c19..19cb26479942 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -106,6 +106,19 @@ static int slpc_send(struct intel_guc_slpc *slpc,
> return intel_guc_send(guc, action, in_len);
> }
>
> +static int host2guc_slpc_set_param(struct intel_guc_slpc *slpc,
> +u32 id, u32 value)
> +{
> + struct slpc_event_input data = {0};
> +
> + data.header.value = SLPC_EVENT(SLPC_EVENT_PARAMETER_SET, 2);
> + data.args[0] = id;
> + data.args[1] = value;
> +
> + return slpc_send(slpc, &data, 4);
as suggested before, use of explicit function like:
static int guc_action_slpc_param(guc, u32 id, u32 value)
{
u32 request[] = {
INTEL_GUC_ACTION_SLPC_REQUEST,
SLPC_EVENT(SLPC_EVENT_PARAMETER_SET, 2),
id,
value,
};
return intel_guc_send(guc, request, ARRAY_SIZE(request));
}
will be simpler/cleaner
> +}
> +
> +
> static bool slpc_running(struct intel_guc_slpc *slpc)
> {
> struct slpc_shared_data *data;
> @@ -134,6 +147,19 @@ static int host2guc_slpc_query_task_state(struct
> intel_guc_slpc *slpc)
> return slpc_send(slpc, &data, 4);
> }
>
> +static int slpc_set_param(struct intel_guc_slpc *slpc, u32 id, u32 value)
> +{
> + struct drm_i915_private *i915 = slpc_to_i915(slpc);
> + GEM_BUG_ON(id >= SLPC_MAX_PARAM);
> +
> + if (host2guc_slpc_set_param(slpc, id, value)) {
> + drm_err(&i915->drm, "Unable to set param %x", id);
missing \n
what about printing value to be set ?
what about printing send error %pe ?
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> static int slpc_read_task_state(struct intel_guc_slpc *slpc)
> {
> return host2guc_slpc_query_task_state(slpc);
> @@ -218,6 +244,74 @@ int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
> return slpc_shared_data_init(slpc);
> }
>
> +/**
> + * intel_guc_slpc_max_freq_set() - Set max frequency limit for SLPC.
> + * @slpc: pointer to intel_guc_slpc.
> + * @val: encoded frequency
what's the encoding ?
> + *
> + * This function will invoke GuC SLPC action to update the max frequency
> + * limit for slice and unslice.
> + *
> + * Return: 0 on success, non-zero error code on failure.
> + */
> +int intel_guc_slpc_set_max_freq(struct intel_guc_slpc *slpc, u32 val)
> +{
> + int ret;
> + struct drm_i915_private *i915 = slpc_to_i915(slpc);
> + intel_wakeref_t wakeref;
> +
> + wakeref = intel_runtime_pm_get(&i915->runtime_pm);
use can use with_intel_runtime_pm(rpm, wakeref)
> +
> + ret = slpc_set_param(slpc,
> +SLPC_PARAM_GLOBAL_MAX_GT_UNSLICE_FREQ_MHZ,
> +val);
> +
> + if (ret) {
> + drm_err(&i915->drm,
> + "Set max frequency unslice returned %d", ret);
missing \n
print error with %pe
but slpc_set_param returns only -EIO ;(
> + ret = -EIO;
> + goto done;
> + }
> +
> +done:
> + intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> + return ret;
> +}
> +
> +/**
> + * intel_guc_slpc_min_freq_set() - Set min frequency limit for SLPC.
> + * @slpc: pointer to intel_guc_slpc.
> + * @val: encoded frequency
> + *
> + * This function will invoke GuC SLPC action to update the min frequency
> + * limit.
> + *
> + * Return: 0 on success, non-zero error code on failure.
> + */
> +int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val)
> +{
> + int ret;
> + struct intel_guc *guc = slpc_to_guc(slpc);
> + struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> + intel_wakeref_t wakeref;
> +
> + wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +
> + ret = slpc_set_param(slpc,
> +SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> +val);
> + if (ret) {
> + drm_err(&i915->drm,
> + "Set min frequency for unslice returned %d", ret);
as above
Michal
> + ret = -EIO;
> + goto done;
> + }
> +
> +done:
> + intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> + return ret;
> +}
> +
> /*
> * intel_guc_slpc_enable() - Start SLPC
> * @slpc: pointer to intel_guc_slpc.
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> index a2643b904165..a473e1ea7c10 100644
> --- a/drivers/gpu/drm/i915/gt/uc/
Re: [Intel-gfx] [PATCH 07/16] drm/i915/guc/slpc: Enable slpc and add related H2G events
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Add methods for interacting with guc for enabling SLPC. Enable
> SLPC after guc submission has been established. GuC load will
s/guc/GuC
> fail if SLPC cannot be successfully initialized. Add various
> helper methods to set/unset the parameters for SLPC. They can
> be set using h2g calls or directly setting bits in the shared
/h2g/H2G
> data structure.
>
> Signed-off-by: Vinay Belgaumkar
> Signed-off-by: Sundaresan Sujaritha
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 221 ++
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 -
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 10 +
> 3 files changed, 231 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index 94e2f19951aa..e579408d1c19 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -18,6 +18,61 @@ static inline struct intel_guc *slpc_to_guc(struct
> intel_guc_slpc *slpc)
> return container_of(slpc, struct intel_guc, slpc);
> }
>
> +static inline struct intel_gt *slpc_to_gt(struct intel_guc_slpc *slpc)
> +{
> + return guc_to_gt(slpc_to_guc(slpc));
> +}
> +
> +static inline struct drm_i915_private *slpc_to_i915(struct intel_guc_slpc
> *slpc)
> +{
> + return (slpc_to_gt(slpc))->i915;
> +}
> +
> +static void slpc_mem_set_param(struct slpc_shared_data *data,
> + u32 id, u32 value)
> +{
> + GEM_BUG_ON(id >= SLPC_MAX_OVERRIDE_PARAMETERS);
> + /* When the flag bit is set, corresponding value will be read
> + * and applied by slpc.
fix format of multi-line comment
s/slpc/SLPC
> + */
> + data->override_params_set_bits[id >> 5] |= (1 << (id % 32));
use __set_bit instead
> + data->override_params_values[id] = value;
> +}
> +
> +static void slpc_mem_unset_param(struct slpc_shared_data *data,
> + u32 id)
> +{
> + GEM_BUG_ON(id >= SLPC_MAX_OVERRIDE_PARAMETERS);
> + /* When the flag bit is unset, corresponding value will not be
> + * read by slpc.
> + */
> + data->override_params_set_bits[id >> 5] &= (~(1 << (id % 32)));
same here
> + data->override_params_values[id] = 0;
> +}
> +
> +static void slpc_mem_task_control(struct slpc_shared_data *data,
> + u64 val, u32 enable_id, u32 disable_id)
hmm, u64 to pass simple tri-state flag ?
> +{
> + /* Enabling a param involves setting the enable_id
> + * to 1 and disable_id to 0. Setting it to default
> + * will unset both enable and disable ids and let
> + * slpc choose it's default values.
fix format + s/slpc/SLPC
> + */
> + if (val == SLPC_PARAM_TASK_DEFAULT) {
> + /* set default */
> + slpc_mem_unset_param(data, enable_id);
> + slpc_mem_unset_param(data, disable_id);
> + } else if (val == SLPC_PARAM_TASK_ENABLED) {
> + /* set enable */
> + slpc_mem_set_param(data, enable_id, 1);
> + slpc_mem_set_param(data, disable_id, 0);
> + } else if (val == SLPC_PARAM_TASK_DISABLED) {
> + /* set disable */
> + slpc_mem_set_param(data, disable_id, 1);
> + slpc_mem_set_param(data, enable_id, 0);
> + }
maybe instead of SLPC_PARAM_TASK_* flags (that btw were confusing me
earlier) you can define 3x small helpers:
static void slpc_mem_set_default(data, enable_id, disable_id);
static void slpc_mem_set_enabled(data, enable_id, disable_id);
static void slpc_mem_set_disabled(data, enable_id, disable_id);
> +}
> +
> static int slpc_shared_data_init(struct intel_guc_slpc *slpc)
> {
> struct intel_guc *guc = slpc_to_guc(slpc);
> @@ -34,6 +89,128 @@ static int slpc_shared_data_init(struct intel_guc_slpc
> *slpc)
> return err;
> }
>
> +/*
> + * Send SLPC event to guc
> + *
> + */
> +static int slpc_send(struct intel_guc_slpc *slpc,
> + struct slpc_event_input *input,
> + u32 in_len)
> +{
> + struct intel_guc *guc = slpc_to_guc(slpc);
> + u32 *action;
> +
> + action = (u32 *)input;
> + action[0] = INTEL_GUC_ACTION_SLPC_REQUEST;
why not just updating input->h2g_action_id ?
> +
> + return intel_guc_send(guc, action, in_len);
> +}
> +
> +static bool slpc_running(struct intel_guc_slpc *slpc)
> +{
> + struct slpc_shared_data *data;
> + u32 slpc_global_state;
> +
> + GEM_BUG_ON(!slpc->vma);
> +
> + drm_clflush_virt_range(slpc->vaddr, sizeof(struct slpc_shared_data));
do you really need to flush all 8K of shared data?
it looks that you only need single u32
> + data = slpc->vaddr;
> +
> + slpc_global_state = data->global_state;
> +
> + return (data->global_state == SLPC_GLOBAL_STATE_RUNNING);
> +}
> +
> +static int host2guc_slpc_query_task_state(struct intel_guc_slpc *slpc)
> +{
> + struct
[PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
From: Frank Wunderlich
HDMI output was broken on mt7623/BPI-R2 in 5.13 because function for
special output selection (mtk_mmsys_ddp_sout_sel) was dropped.
This function wrote 3 registers at one time and so it is not compatible
with the array-approach.
I re-added the old function and call this after the default routing table
was applied. This default routing table can still be used as the only
connection handled by mmsys on BPI-R2 is OVL0->RDMA0 and this is already
present in the default routing table
Fixes: 440147639ac7 ("soc: mediatek: mmsys: Use an array for setting the
routing registers")
Signed-off-by: Frank Wunderlich
---
drivers/soc/mediatek/mtk-mmsys.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index 080660ef11bf..f91b7fdd417a 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -57,6 +57,25 @@ struct mtk_mmsys {
const struct mtk_mmsys_driver_data *data;
};
+static void mtk_mmsys_ddp_sout_sel(struct device *dev,
+ enum mtk_ddp_comp_id cur,
+ enum mtk_ddp_comp_id next)
+{
+ struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+
+ if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
+ writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
+ mmsys->regs + DISP_REG_CONFIG_OUT_SEL);
+ } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
+ writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI,
+ mmsys->regs + DISP_REG_CONFIG_OUT_SEL);
+ writel_relaxed(DSI_SEL_IN_RDMA,
+ mmsys->regs + DISP_REG_CONFIG_DSI_SEL);
+ writel_relaxed(DPI_SEL_IN_BLS,
+ mmsys->regs + DISP_REG_CONFIG_DPI_SEL);
+ }
+}
+
void mtk_mmsys_ddp_connect(struct device *dev,
enum mtk_ddp_comp_id cur,
enum mtk_ddp_comp_id next)
@@ -71,6 +90,8 @@ void mtk_mmsys_ddp_connect(struct device *dev,
reg = readl_relaxed(mmsys->regs + routes[i].addr) |
routes[i].val;
writel_relaxed(reg, mmsys->regs + routes[i].addr);
}
+
+ mtk_mmsys_ddp_sout_sel(dev, cur, next);
}
EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect);
--
2.25.1
Re: [PATCH 06/16] drm/i915/guc/slpc: Allocate, initialize and release slpc
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Allocate data structures for SLPC and functions for
> initializing on host side.
>
> Signed-off-by: Vinay Belgaumkar
> Signed-off-by: Sundaresan Sujaritha
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 11 +++
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 36 -
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 20
> 3 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 9d61b2d54de4..82863a9bc8e8 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -336,6 +336,12 @@ int intel_guc_init(struct intel_guc *guc)
> goto err_ct;
> }
>
> + if (intel_guc_slpc_is_used(guc)) {
> + ret = intel_guc_slpc_init(&guc->slpc);
> + if (ret)
> + goto err_submission;
> + }
> +
> /* now that everything is perma-pinned, initialize the parameters */
> guc_init_params(guc);
>
> @@ -346,6 +352,8 @@ int intel_guc_init(struct intel_guc *guc)
>
> return 0;
>
> +err_submission:
> + intel_guc_submission_fini(guc);
> err_ct:
> intel_guc_ct_fini(&guc->ct);
> err_ads:
> @@ -368,6 +376,9 @@ void intel_guc_fini(struct intel_guc *guc)
>
> i915_ggtt_disable_guc(gt->ggtt);
>
> + if (intel_guc_slpc_is_used(guc))
> + intel_guc_slpc_fini(&guc->slpc);
> +
> if (intel_guc_submission_is_used(guc))
> intel_guc_submission_fini(guc);
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index c1f569d2300d..94e2f19951aa 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -4,11 +4,41 @@
> * Copyright © 2020 Intel Corporation
> */
>
> +#include
hmm, what exactly is needed from this header ?
> +
> +#include "gt/intel_gt.h"
> +#include "gt/intel_rps.h"
> +
> +#include "i915_drv.h"
> #include "intel_guc_slpc.h"
> +#include "intel_pm.h"
> +
> +static inline struct intel_guc *slpc_to_guc(struct intel_guc_slpc *slpc)
> +{
> + return container_of(slpc, struct intel_guc, slpc);
> +}
> +
> +static int slpc_shared_data_init(struct intel_guc_slpc *slpc)
> +{
> + struct intel_guc *guc = slpc_to_guc(slpc);
> + int err;
> + u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data));
move err decl here
> +
> + err = intel_guc_allocate_and_map_vma(guc, size, &slpc->vma,
> &slpc->vaddr);
> + if (unlikely(err)) {
> + DRM_ERROR("Failed to allocate slpc struct (err=%d)\n", err);
s/slpc/SLPC
and use drm_err instead
and you may also want to print error as %pe
> + i915_vma_unpin_and_release(&slpc->vma, I915_VMA_RELEASE_MAP);
do you really need this ?
> + return err;
> + }
> +
> + return err;
> +}
>
> int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
> {
> - return 0;
> + GEM_BUG_ON(slpc->vma);
> +
> + return slpc_shared_data_init(slpc);
> }
>
> /*
> @@ -31,4 +61,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>
> void intel_guc_slpc_fini(struct intel_guc_slpc *slpc)
> {
> + if (!slpc->vma)
> + return;
> +
> + i915_vma_unpin_and_release(&slpc->vma, I915_VMA_RELEASE_MAP);
> }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> index 98036459a1a3..a2643b904165 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> @@ -3,12 +3,32 @@
> *
> * Copyright © 2020 Intel Corporation
> */
> +
should be fixed in earlier patch
> #ifndef _INTEL_GUC_SLPC_H_
> #define _INTEL_GUC_SLPC_H_
>
> +#include
> #include "intel_guc_slpc_fwif.h"
>
> struct intel_guc_slpc {
> + /*Protects access to vma and SLPC actions */
hmm, missing mutex ;)
> + struct i915_vma *vma;
> + void *vaddr;
no need to be void, define it as ptr to slpc_shared_data
> +
> + /* platform frequency limits */
> + u32 min_freq;
> + u32 rp0_freq;
> + u32 rp1_freq;
> +
> + /* frequency softlimits */
> + u32 min_freq_softlimit;
> + u32 max_freq_softlimit;
> +
> + struct {
> + u32 param_id;
> + u32 param_value;
> + u32 param_override;
> + } debug;
can you add all these extra fields in patches which will need them?
Michal
> };
>
> int intel_guc_slpc_init(struct intel_guc_slpc *slpc);
>
Re: [PATCH] dim/drm-misc: Add rule to not push patches with issues
On Fri, Jul 09, 2021 at 10:11:16AM +0200, Daniel Vetter wrote: > We kinda left this out, and I like the wording from the drm-intel > side, so add that. Motivated by a discussion with Christian. > > Cc: Christian König > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Signed-off-by: Daniel Vetter Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH 1/1] drm: bridge: Mark deprecated operations in drm_bridge_funcs
On Sat, Jul 10, 2021 at 10:42:40AM +0200, Sam Ravnborg wrote: > drm_bridge_funcs includes several duplicated operations as atomic > variants has been added over time. > New bridge drivers shall use the atomic variants - mark the deprecated > operations to try to avoid usage in new bridge drivers. > > Signed-off-by: Sam Ravnborg > Cc: Laurent Pinchart > Cc: Andrzej Hajda > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH 05/16] drm/i915/guc/slpc: Adding slpc communication interfaces
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Replicate the SLPC header file in GuC for the most part. There are
what you mean by "replicate" here?
> some SLPC mode based parameters which haven't been included since
> we are not using them.
>
> Signed-off-by: Vinay Belgaumkar
> Signed-off-by: Sundaresan Sujaritha
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 +
> drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 2 +
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 2 +
> .../gpu/drm/i915/gt/uc/intel_guc_slpc_fwif.h | 255 ++
> 4 files changed, 263 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_fwif.h
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index b9a809f2d221..9d61b2d54de4 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -202,11 +202,15 @@ static u32 guc_ctl_debug_flags(struct intel_guc *guc)
>
> static u32 guc_ctl_feature_flags(struct intel_guc *guc)
> {
> + struct intel_gt *gt = guc_to_gt(guc);
> u32 flags = 0;
>
> if (!intel_guc_submission_is_used(guc))
> flags |= GUC_CTL_DISABLE_SCHEDULER;
>
> + if (intel_uc_uses_guc_slpc(>->uc))
> + flags |= GUC_CTL_ENABLE_SLPC;
> +
> return flags;
> }
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> index 94bb1ca6f889..19e2504d7a36 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> @@ -114,6 +114,8 @@
> #define GUC_ADS_ADDR_SHIFT 1
> #define GUC_ADS_ADDR_MASK (0xF << GUC_ADS_ADDR_SHIFT)
>
> +#define GUC_CTL_ENABLE_SLPCBIT(2)
this should be defined closer to GUC_CTL_FEATURE
> +
> #define GUC_CTL_MAX_DWORDS (SOFT_SCRATCH_COUNT - 2) /* [1..14] */
>
> /* Generic GT SysInfo data types */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> index 74fd86769163..98036459a1a3 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> @@ -6,6 +6,8 @@
> #ifndef _INTEL_GUC_SLPC_H_
> #define _INTEL_GUC_SLPC_H_
>
> +#include "intel_guc_slpc_fwif.h"
doesn't seem to be needed right now
> +
> struct intel_guc_slpc {
> };
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_fwif.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_fwif.h
> new file mode 100644
> index ..2a5e71428374
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc_fwif.h
I've started to move all pure ABI definitions to files in abi/ folder,
leaving in guc_fwif.h only our next level helpers/wrappers.
Can you move these SLPC definition there too ? maybe as dedicated:
abi/guc_slpc_abi.h
> @@ -0,0 +1,255 @@
> +/*
> + * SPDX-License-Identifier: MIT
use proper format
> + *
> + * Copyright © 2020 Intel Corporation
2021
> + */
> +#ifndef _INTEL_GUC_SLPC_FWIF_H_
> +#define _INTEL_GUC_SLPC_FWIF_H_
> +
> +#include
> +
> +/* This file replicates the header in GuC code for handling SLPC related
> + * data structures and sizes
> + */
use proper format for multi-line comments:
/*
* blah blah
* blah blah
*/
> +
> +/* SLPC exposes certain parameters for global configuration by the host.
> + * These are referred to as override parameters, because in most cases
> + * the host will not need to modify the default values used by SLPC.
> + * SLPC remembers the default values which allows the host to easily restore
> + * them by simply unsetting the override. The host can set or unset override
> + * parameters during SLPC (re-)initialization using the SLPC Reset event.
> + * The host can also set or unset override parameters on the fly using the
> + * Parameter Set and Parameter Unset events
> + */
> +#define SLPC_MAX_OVERRIDE_PARAMETERS 256
> +#define SLPC_OVERRIDE_BITFIELD_SIZE \
> + (SLPC_MAX_OVERRIDE_PARAMETERS / 32)
> +
> +#define SLPC_PAGE_SIZE_BYTES 4096
> +#define SLPC_CACHELINE_SIZE_BYTES64
> +#define SLPC_SHARE_DATA_SIZE_BYTE_HEADER SLPC_CACHELINE_SIZE_BYTES
> +#define SLPC_SHARE_DATA_SIZE_BYTE_PLATFORM_INFO
> SLPC_CACHELINE_SIZE_BYTES
> +#define SLPC_SHARE_DATA_SIZE_BYTE_TASK_STATE SLPC_CACHELINE_SIZE_BYTES
> +#define SLPC_SHARE_DATA_MODE_DEFN_TABLE_SIZE SLPC_PAGE_SIZE_BYTES
can you put some simply diagram that would describe this layout ?
> +
> +#define SLPC_SHARE_DATA_SIZE_BYTE_MAX(2 *
> SLPC_PAGE_SIZE_BYTES)
> +
> +/* Cacheline size aligned (Total size needed for
> + * SLPM_KMD_MAX_OVERRIDE_PARAMETERS=256 is 1088 bytes)
> + */
> +#define SLPC_SHARE_DATA_SIZE_BYTE_PARAM
> (SLPC_MAX_OVERRIDE_PARAMETERS * 4) \
> + +
> ((SLPC_MAX_OVERRIDE_PARAMETERS / 32) * 4)) \
> +
Re: [Intel-gfx] [PATCH 04/16] drm/i915/guc/slpc: Lay out slpc init/enable/disable/fini
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Declare header and source files for SLPC, along with init and
> enable/disable function templates.
later you claim that "disable" is not needed
>
> Signed-off-by: Vinay Belgaumkar
> Signed-off-by: Sundaresan Sujaritha
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/gt/uc/intel_guc.h | 2 ++
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 34 +
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h | 16 ++
> 4 files changed, 53 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index ab7679957623..d8eac4468df9 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -186,6 +186,7 @@ i915-y += gt/uc/intel_uc.o \
> gt/uc/intel_guc_fw.o \
> gt/uc/intel_guc_log.o \
> gt/uc/intel_guc_log_debugfs.o \
> + gt/uc/intel_guc_slpc.o \
> gt/uc/intel_guc_submission.o \
> gt/uc/intel_huc.o \
> gt/uc/intel_huc_debugfs.o \
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index e5a456918b88..0dbbd9cf553f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -15,6 +15,7 @@
> #include "intel_guc_ct.h"
> #include "intel_guc_log.h"
> #include "intel_guc_reg.h"
> +#include "intel_guc_slpc.h"
> #include "intel_uc_fw.h"
> #include "i915_utils.h"
> #include "i915_vma.h"
> @@ -30,6 +31,7 @@ struct intel_guc {
> struct intel_uc_fw fw;
> struct intel_guc_log log;
> struct intel_guc_ct ct;
> + struct intel_guc_slpc slpc;
>
> /* Global engine used to submit requests to GuC */
> struct i915_sched_engine *sched_engine;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> new file mode 100644
> index ..c1f569d2300d
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -0,0 +1,34 @@
> +/*
> + * SPDX-License-Identifier: MIT
SPDX tag shall be in very first line, for .c:
// SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2020 Intel Corporation
2021
> + */
> +
> +#include "intel_guc_slpc.h"
> +
> +int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
> +{
> + return 0;
> +}
> +
> +/*
> + * intel_guc_slpc_enable() - Start SLPC
> + * @slpc: pointer to intel_guc_slpc.
> + *
> + * SLPC is enabled by setting up the shared data structure and
> + * sending reset event to GuC SLPC. Initial data is setup in
> + * intel_guc_slpc_init. Here we send the reset event. We do
> + * not currently need a slpc_disable since this is taken care
> + * of automatically when a reset/suspend occurs and the guc
s/guc/GuC
> + * channels are destroyed.
you mean CTB ?
> + *
> + * Return: 0 on success, non-zero error code on failure.
> + */
> +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
> +{
> + return 0;
> +}
> +
> +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc)
> +{
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> new file mode 100644
> index ..74fd86769163
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> @@ -0,0 +1,16 @@
> +/*
> + * SPDX-License-Identifier: MIT
SPDX tag shall be in very first line, for .h:
/* SPDX-License-Identifier: MIT */
> + *
> + * Copyright © 2020 Intel Corporation
2021
> + */
> +#ifndef _INTEL_GUC_SLPC_H_
> +#define _INTEL_GUC_SLPC_H_
> +
> +struct intel_guc_slpc {
> +};
move all data definitions to intel_guc_slpc_types.h and include it here
> +
> +int intel_guc_slpc_init(struct intel_guc_slpc *slpc);
> +int intel_guc_slpc_enable(struct intel_guc_slpc *slpc);
> +void intel_guc_slpc_fini(struct intel_guc_slpc *slpc);
> +
> +#endif
>
and as suggested in comment to 2/14 you should likely move this patch to
the front of the series
Michal
Re: [PATCH 02/16] drm/i915/guc/slpc: Initial definitions for slpc
Hi Vinay,
On 10.07.2021 03:20, Vinay Belgaumkar wrote:
> Add macros to check for slpc support. This feature is currently supported
> for gen12+ and enabled whenever guc submission is enabled/selected.
please try to use consistent names across all patches:
s/slpc/SLPC
s/gen12/Gen12
s/guc/GuC
>
> Signed-off-by: Vinay Belgaumkar
> Signed-off-by: Sundaresan Sujaritha
> Signed-off-by: Daniele Ceraolo Spurio
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc.c| 1 +
> drivers/gpu/drm/i915/gt/uc/intel_guc.h| 2 ++
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 21 +++
> .../gpu/drm/i915/gt/uc/intel_guc_submission.h | 16 ++
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 6 --
> drivers/gpu/drm/i915/gt/uc/intel_uc.h | 1 +
> 6 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 979128e28372..b9a809f2d221 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -157,6 +157,7 @@ void intel_guc_init_early(struct intel_guc *guc)
> intel_guc_ct_init_early(&guc->ct);
> intel_guc_log_init_early(&guc->log);
> intel_guc_submission_init_early(guc);
> + intel_guc_slpc_init_early(guc);
>
> mutex_init(&guc->send_mutex);
> spin_lock_init(&guc->irq_lock);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index 5d94cf482516..e5a456918b88 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -57,6 +57,8 @@ struct intel_guc {
>
> bool submission_supported;
> bool submission_selected;
> + bool slpc_supported;
> + bool slpc_selected;
>
> struct i915_vma *ads_vma;
> struct __guc_ads_blob *ads_blob;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 9c102bf0c8e3..e2644a05f298 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -2351,6 +2351,27 @@ void intel_guc_submission_init_early(struct intel_guc
> *guc)
> guc->submission_selected = __guc_submission_selected(guc);
> }
>
> +static bool __guc_slpc_supported(struct intel_guc *guc)
hmm, easy to confuse with intel_guc_slpc_is_supported, so maybe:
__detect_slpc_supported()
(yes, I know you were following code above)
> +{
> + /* GuC slpc is unavailable for pre-Gen12 */
s/slpc/SLPC
> + return guc->submission_supported &&
> + GRAPHICS_VER(guc_to_gt(guc)->i915) >= 12;
> +}
> +
> +static bool __guc_slpc_selected(struct intel_guc *guc)
> +{
> + if (!intel_guc_slpc_is_supported(guc))
> + return false;
> +
> + return guc->submission_selected;
> +}
> +
> +void intel_guc_slpc_init_early(struct intel_guc *guc)
> +{
> + guc->slpc_supported = __guc_slpc_supported(guc);
> + guc->slpc_selected = __guc_slpc_selected(guc);
> +}
in patch 4/16 you are introducing intel_guc_slpc.c|h so to have proper
encapsulation better to define this function as
void intel_guc_slpc_init_early(struct intel_guc_slpc *slpc) { }
and move it to intel_guc_slpc.c
> +
> static inline struct intel_context *
> g2h_context_lookup(struct intel_guc *guc, u32 desc_idx)
> {
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> index be767eb6ff71..7ae5fd052faf 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
> @@ -13,6 +13,7 @@
> struct drm_printer;
> struct intel_engine_cs;
>
> +void intel_guc_slpc_init_early(struct intel_guc *guc);
it really does not belong to this .h
> void intel_guc_submission_init_early(struct intel_guc *guc);
> int intel_guc_submission_init(struct intel_guc *guc);
> void intel_guc_submission_enable(struct intel_guc *guc);
> @@ -50,4 +51,19 @@ static inline bool intel_guc_submission_is_used(struct
> intel_guc *guc)
> return intel_guc_is_used(guc) && intel_guc_submission_is_wanted(guc);
> }
>
> +static inline bool intel_guc_slpc_is_supported(struct intel_guc *guc)
> +{
> + return guc->slpc_supported;
> +}
> +
> +static inline bool intel_guc_slpc_is_wanted(struct intel_guc *guc)
> +{
> + return guc->slpc_selected;
> +}
> +
> +static inline bool intel_guc_slpc_is_used(struct intel_guc *guc)
> +{
> + return intel_guc_submission_is_used(guc) &&
> intel_guc_slpc_is_wanted(guc);
> +}
did you try to define them in intel_guc_slpc.h ?
note that to avoid circular dependencies you can define slpc struct in
intel_guc_slpc_types.h and then
in intel_guc.h:
#include "intel_guc_slpc_types.h" instead of intel_guc_slpc.h
in intel_guc_slpc.h:
#include "intel_guc.h"
#include "intel_guc_slpc_types.h"
#include "intel_guc_sub
Re: [PATCH 14/16] drm/i915/guc/slpc: Sysfs hooks for slpc
Hi Vinay,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next
tegra-drm/drm/tegra/for-next drm/drm-next v5.13 next-20210709]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Vinay-Belgaumkar/Enable-GuC-based-power-management-features/20210710-092520
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-a004-20210709 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
#
https://github.com/0day-ci/linux/commit/8388422991b4e0e4da460328634a7ec1d278de6a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Vinay-Belgaumkar/Enable-GuC-based-power-management-features/20210710-092520
git checkout 8388422991b4e0e4da460328634a7ec1d278de6a
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/gt/intel_rps.c:1969:5: warning: no previous prototype
>> for 'intel_rps_read_punit_req' [-Wmissing-prototypes]
1969 | u32 intel_rps_read_punit_req(struct intel_rps *rps)
| ^~~~
>> drivers/gpu/drm/i915/gt/intel_rps.c:1978:5: warning: no previous prototype
>> for 'intel_rps_get_req' [-Wmissing-prototypes]
1978 | u32 intel_rps_get_req(struct intel_rps *rps, u32 pureq)
| ^
>> drivers/gpu/drm/i915/gt/intel_rps.c:1985:5: warning: no previous prototype
>> for 'intel_rps_read_punit_req_frequency' [-Wmissing-prototypes]
1985 | u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps)
| ^~
vim +/intel_rps_read_punit_req +1969 drivers/gpu/drm/i915/gt/intel_rps.c
1968
> 1969 u32 intel_rps_read_punit_req(struct intel_rps *rps)
1970 {
1971 struct intel_uncore *uncore = rps_to_uncore(rps);
1972
1973 u32 pureq = intel_uncore_read(uncore, GEN6_RPNSWREQ);
1974
1975 return pureq;
1976 }
1977
> 1978 u32 intel_rps_get_req(struct intel_rps *rps, u32 pureq)
1979 {
1980 u32 req = pureq >> GEN9_SW_REQ_UNSLICE_RATIO_SHIFT;
1981
1982 return req;
1983 }
1984
> 1985 u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps)
1986 {
1987 u32 freq = intel_rps_get_req(rps,
intel_rps_read_punit_req(rps));
1988
1989 return intel_gpu_freq(rps, freq);
1990 }
1991
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[Bug 212469] plymouth animation freezes during shutdown
https://bugzilla.kernel.org/show_bug.cgi?id=212469 --- Comment #11 from Arnas (arnasz...@gmail.com) --- The problem was not introduced with 5.10, 5.10 always worked fine. The problem appeared with kernel 5.11. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH] drm/dp: Move panel DP AUX backlight support to drm_dp_helper
Hi Doug,
On 10-07-2021 03:59, Douglas Anderson wrote:
We were getting a depmod error:
depmod: ERROR: Cycle detected: drm_kms_helper -> drm ->
drm_kms_helper
It looks like the rule is that drm_kms_helper can call into drm, but
drm can't call into drm_kms_helper. That means we've got to move the
DP AUX backlight support into drm_dp_helper.
NOTE: as part of this, I didn't try to do any renames of the main
registration function. Even though it's in the drm_dp_helper, it still
feels very parallel to drm_panel_of_backlight().
Fixes: 10f7b40e4f30 ("drm/panel: add basic DP AUX backlight support")
Reported-by: Ville Syrjälä
Signed-off-by: Douglas Anderson
---
Note that I've compile tested this, but I don't have a device setup
yet that uses this code. Since the code is basically the same as it
was this should be OK, but if Rajeev could confirm that nothing is
broken that'd be nice.
drivers/gpu/drm/drm_dp_helper.c | 108
drivers/gpu/drm/drm_panel.c | 108
include/drm/drm_dp_helper.h | 3 +
include/drm/drm_panel.h | 8 ---
4 files changed, 111 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c
b/drivers/gpu/drm/drm_dp_helper.c
index 24bbc710c825..5b1c772e8f38 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -33,9 +33,17 @@
#include
#include
#include
+#include
#include "drm_crtc_helper_internal.h"
+struct dp_aux_backlight {
+ struct backlight_device *base;
+ struct drm_dp_aux *aux;
+ struct drm_edp_backlight_info info;
+ bool enabled;
+};
+
/**
* DOC: dp helpers
*
@@ -3462,3 +3470,103 @@ drm_edp_backlight_init(struct drm_dp_aux *aux,
struct drm_edp_backlight_info *bl
return 0;
}
EXPORT_SYMBOL(drm_edp_backlight_init);
+
+static int dp_aux_backlight_update_status(struct backlight_device *bd)
+{
+ struct dp_aux_backlight *bl = bl_get_data(bd);
+ u16 brightness = backlight_get_brightness(bd);
+ int ret = 0;
+
+ if (!backlight_is_blank(bd)) {
+ if (!bl->enabled) {
+ drm_edp_backlight_enable(bl->aux, &bl->info,
brightness);
+ bl->enabled = true;
+ return 0;
+ }
+ ret = drm_edp_backlight_set_level(bl->aux, &bl->info,
brightness);
+ } else {
+ if (bl->enabled) {
+ drm_edp_backlight_disable(bl->aux, &bl->info);
+ bl->enabled = false;
+ }
+ }
+
+ return ret;
+}
+
+static const struct backlight_ops dp_aux_bl_ops = {
+ .update_status = dp_aux_backlight_update_status,
+};
+
+/**
+ * drm_panel_dp_aux_backlight - create and use DP AUX backlight
+ * @panel: DRM panel
+ * @aux: The DP AUX channel to use
+ *
+ * Use this function to create and handle backlight if your panel
+ * supports backlight control over DP AUX channel using DPCD
+ * registers as per VESA's standard backlight control interface.
+ *
+ * When the panel is enabled backlight will be enabled after a
+ * successful call to &drm_panel_funcs.enable()
+ *
+ * When the panel is disabled backlight will be disabled before the
+ * call to &drm_panel_funcs.disable().
+ *
+ * A typical implementation for a panel driver supporting backlight
+ * control over DP AUX will call this function at probe time.
+ * Backlight will then be handled transparently without requiring
+ * any intervention from the driver.
+ *
+ * drm_panel_dp_aux_backlight() must be called after the call to
drm_panel_init().
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct
drm_dp_aux *aux)
+{
+ struct dp_aux_backlight *bl;
+ struct backlight_properties props = { 0 };
+ u16 current_level;
+ u8 current_mode;
+ u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+ int ret;
+
+ if (!panel || !panel->dev || !aux)
+ return -EINVAL;
+
+ ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd,
+ EDP_DISPLAY_CTL_CAP_SIZE);
+ if (ret < 0)
+ return ret;
+
+ if (!drm_edp_backlight_supported(edp_dpcd)) {
+ DRM_DEV_INFO(panel->dev, "DP AUX backlight is not supported\n");
+ return 0;
+ }
+
+ bl = devm_kzalloc(panel->dev, sizeof(*bl), GFP_KERNEL);
+ if (!bl)
+ return -ENOMEM;
+
+ bl->aux = aux;
+
+ ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
+¤t_level, ¤t_mode);
+ if (ret < 0)
+ return ret;
+
+ props.type = BACKLIGHT_RAW;
+ props.brightness = current_level;
+ props.max_brightness = bl->info.max;
+
+ bl->base = devm_backlight_device_register(panel->dev,
"dp_aux_backlight",
+ panel->dev, bl,
+
Re: [PATCH 1/2] dt-bindings: display/panel: Add Innolux EJ030NA
[...] I am not sure; the doc states that this (additionalProperties: false) "can't be used in case where another schema is referenced", which is the case here, as we include "panel-common.yaml". This DT schema already list all relevant properties like: backlight: true So "additionalProperties: false" tells that no other properties are allowed other than the listed properties. To my best understanding unevaluatedProperties: false is less strict and should be used if one does not list all possilbe properties. This could be the case for a panel haging below a SPI controller as in this case. So in other words giving this some extra thought I think unevaluatedProperties: false is OK here. A panel below a SPI controller would have all its SPI-specific properties covered by spi-controller.yaml, I believe? So maybe "additionalProperties: false" would work? In any case, if I use "additionalProperties: false", "make dt_binding_check" complains that my example's "spi-max-frequency" property is not covered. So maybe you are right. So my r-b is ok if you keep it as it. PS. Where do you guys hang out with the downfall of freenode - somewhere on oftc? We moved to #opendingux on Libera. Cheers, -Paul
Re: [PATCH] drm/ingenic: Convert to Linux IRQ interfaces
Hi,
Le sam., juil. 10 2021 at 08:33:47 +0200, Sam Ravnborg
a écrit :
Hi Thomas,
On Tue, Jul 06, 2021 at 09:44:09AM +0200, Thomas Zimmermann wrote:
Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
don't benefit from using it.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index c296472164d9..a09b7da21b53 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -33,7 +33,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -799,8 +798,6 @@ static const struct drm_driver
ingenic_drm_driver_data = {
.fops = &ingenic_drm_fops,
.gem_create_object = ingenic_drm_gem_create_object,
DRM_GEM_CMA_DRIVER_OPS,
-
- .irq_handler= ingenic_drm_irq_handler,
};
static const struct drm_plane_funcs
ingenic_drm_primary_plane_funcs = {
@@ -1098,7 +1095,7 @@ static int ingenic_drm_bind(struct device
*dev, bool has_components)
encoder->possible_clones = clone_mask;
}
- ret = drm_irq_install(drm, irq);
+ ret = request_irq(irq, ingenic_drm_irq_handler, 0,
drm->driver->name, drm);
Can't you use the devm variant?
if (ret) {
dev_err(dev, "Unable to install IRQ handler\n");
return ret;
@@ -1192,14 +1189,18 @@ static void ingenic_drm_unbind(struct
device *dev)
{
struct ingenic_drm *priv = dev_get_drvdata(dev);
struct clk *parent_clk = clk_get_parent(priv->pix_clk);
+ struct drm_device *drm = &priv->drm;
+ struct platform_device *pdev = to_platform_device(drm->dev);
+
+ free_irq(platform_get_irq(pdev, 0), drm);
The driver was missing a drm_irq_uninstall() so the above code is
actually a small bug-fix. It should be mentioned in the changelog.
With this fixed:
Reviewed-by: Sam Ravnborg
Note: I expect Paul to review too and apply.
I wasn't Cc'd? :(
-Paul
Sam
clk_notifier_unregister(parent_clk, &priv->clock_nb);
if (priv->lcd_clk)
clk_disable_unprepare(priv->lcd_clk);
clk_disable_unprepare(priv->pix_clk);
- drm_dev_unregister(&priv->drm);
- drm_atomic_helper_shutdown(&priv->drm);
+ drm_dev_unregister(drm);
+ drm_atomic_helper_shutdown(drm);
}
static const struct component_master_ops ingenic_master_ops = {
--
2.32.0
Re: [PATCH 1/2] dt-bindings: display/panel: Add Innolux EJ030NA
Hi Paul, > > > + backlight: true > > > + port: true > > > + power-supply: true > > > + reg: true > > > + reset-gpios: true > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - power-supply > > > + - reset-gpios > > > + > > > +unevaluatedProperties: false > > I had expected: > > additionalProperties: false > > > > With this fixed: > > Reviewed-by: Sam Ravnborg > > I am not sure; the doc states that this (additionalProperties: false) "can't > be used in case where another schema is referenced", which is the case here, > as we include "panel-common.yaml". This DT schema already list all relevant properties like: backlight: true So "additionalProperties: false" tells that no other properties are allowed other than the listed properties. To my best understanding unevaluatedProperties: false is less strict and should be used if one does not list all possilbe properties. This could be the case for a panel haging below a SPI controller as in this case. So in other words giving this some extra thought I think unevaluatedProperties: false is OK here. So my r-b is ok if you keep it as it. PS. Where do you guys hang out with the downfall of freenode - somewhere on oftc? Sam
Re: [PATCH 1/2] dt-bindings: display/panel: Add Innolux EJ030NA
Hi Sam, thanks for the review.
Le sam., juil. 10 2021 at 08:14:43 +0200, Sam Ravnborg
a écrit :
Hi Paul,
On Fri, Jun 25, 2021 at 01:10:44PM +0100, Paul Cercueil wrote:
Add binding for the Innolux EJ030NA panel, which is a 320x480 3.0"
4:3
24-bit TFT LCD panel with non-square pixels and a delta-RGB 8-bit
interface.
Signed-off-by: Paul Cercueil
---
.../display/panel/innolux,ej030na.yaml| 62
+++
1 file changed, 62 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml
diff --git
a/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml
b/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml
new file mode 100644
index ..cda36c04e85c
--- /dev/null
+++
b/Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id:
http://devicetree.org/schemas/display/panel/innolux,ej030na.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Innolux EJ030NA 3.0" (320x480 pixels) 24-bit TFT LCD panel
+
+description: |
+ The panel must obey the rules for a SPI slave device as
specified in
+ spi/spi-controller.yaml
+
+maintainers:
+ - Paul Cercueil
+
+allOf:
+ - $ref: panel-common.yaml#
+
+properties:
+ compatible:
+const: innolux,ej030na
+
+ backlight: true
+ port: true
+ power-supply: true
+ reg: true
+ reset-gpios: true
+
+required:
+ - compatible
+ - reg
+ - power-supply
+ - reset-gpios
+
+unevaluatedProperties: false
I had expected:
additionalProperties: false
With this fixed:
Reviewed-by: Sam Ravnborg
I am not sure; the doc states that this (additionalProperties: false)
"can't be used in case where another schema is referenced", which is
the case here, as we include "panel-common.yaml".
Cheers,
-Paul
+
+examples:
+ - |
+#include
+
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+panel@0 {
+compatible = "innolux,ej030na";
+reg = <0>;
+
+spi-max-frequency = <1000>;
+
+reset-gpios = <&gpe 4 GPIO_ACTIVE_LOW>;
+power-supply = <&lcd_power>;
+
+backlight = <&backlight>;
+
+port {
+panel_input: endpoint {
+remote-endpoint = <&panel_output>;
+};
+};
+};
+};
--
2.30.2
[PATCH] dma-buf: Delete the DMA-BUF attachment sysfs statistics
The DMA-BUF attachment statistics form a subset of the DMA-BUF
sysfs statistics that recently merged to the drm-misc tree.
Since there has been a reported a performance regression due to the
overhead of sysfs directory creation/teardown during
dma_buf_attach()/dma_buf_detach(), this patch deletes the DMA-BUF
attachment statistics from sysfs.
Fixes: bdb8d06dfefd (dmabuf: Add the capability to expose DMA-BUF stats
in sysfs)
Signed-off-by: Hridya Valsaraju
---
Hello all,
One of our partners recently reported a perf regression in a driver
which was being caused due to the overhead of setup/teardown of the
sysfs attachment statistics in the dma_buf_attach()/dma_buf_detach()
invocations. Since the driver's latency requirements were of the order
of microseconds(~100us), the overhead was significant.
Since this indicates that the solution might not work well for
all DMA-BUF importers, I think the right thing to do might be to delete
the same before it reaches upstream and becomes ABI :( I apologize for
the inconvenience.
This patch is based on the drm-misc-next branch. Please feel free to
let me know if you would prefer that I send a full revert and new patch that
adds the rest of the statistics.
Regards,
Hridya
.../ABI/testing/sysfs-kernel-dmabuf-buffers | 28
drivers/dma-buf/dma-buf-sysfs-stats.c | 140 +-
drivers/dma-buf/dma-buf-sysfs-stats.h | 27
drivers/dma-buf/dma-buf.c | 16 --
include/linux/dma-buf.h | 17 ---
5 files changed, 4 insertions(+), 224 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
index a243984ed420..5d3bc997dc64 100644
--- a/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
+++ b/Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers
@@ -22,31 +22,3 @@ KernelVersion: v5.13
Contact: Hridya Valsaraju
Description: This file is read-only and specifies the size of the DMA-BUF in
bytes.
-
-What: /sys/kernel/dmabuf/buffers//attachments
-Date: May 2021
-KernelVersion: v5.13
-Contact: Hridya Valsaraju
-Description: This directory will contain subdirectories representing every
- attachment of the DMA-BUF.
-
-What:
/sys/kernel/dmabuf/buffers//attachments/
-Date: May 2021
-KernelVersion: v5.13
-Contact: Hridya Valsaraju
-Description: This directory will contain information on the attached device
- and the number of current distinct device mappings.
-
-What:
/sys/kernel/dmabuf/buffers//attachments//device
-Date: May 2021
-KernelVersion: v5.13
-Contact: Hridya Valsaraju
-Description: This file is read-only and is a symlink to the attached device's
- sysfs entry.
-
-What:
/sys/kernel/dmabuf/buffers//attachments//map_counter
-Date: May 2021
-KernelVersion: v5.13
-Contact: Hridya Valsaraju
-Description: This file is read-only and contains a map_counter indicating the
- number of distinct device mappings of the attachment.
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c
b/drivers/dma-buf/dma-buf-sysfs-stats.c
index a2638e84199c..053baadcada9 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
@@ -40,14 +40,11 @@
*
* * ``/sys/kernel/dmabuf/buffers//exporter_name``
* * ``/sys/kernel/dmabuf/buffers//size``
- * *
``/sys/kernel/dmabuf/buffers//attachments//device``
- * *
``/sys/kernel/dmabuf/buffers//attachments//map_counter``
*
- * The information in the interface can also be used to derive per-exporter and
- * per-device usage statistics. The data from the interface can be gathered
- * on error conditions or other important events to provide a snapshot of
- * DMA-BUF usage. It can also be collected periodically by telemetry to monitor
- * various metrics.
+ * The information in the interface can also be used to derive per-exporter
+ * statistics. The data from the interface can be gathered on error conditions
+ * or other important events to provide a snapshot of DMA-BUF usage.
+ * It can also be collected periodically by telemetry to monitor various
metrics.
*
* Detailed documentation about the interface is present in
* Documentation/ABI/testing/sysfs-kernel-dmabuf-buffers.
@@ -121,120 +118,6 @@ static struct kobj_type dma_buf_ktype = {
.default_groups = dma_buf_stats_default_groups,
};
-#define to_dma_buf_attach_entry_from_kobj(x) container_of(x, struct
dma_buf_attach_sysfs_entry, kobj)
-
-struct dma_buf_attach_stats_attribute {
- struct attribute attr;
- ssize_t (*show)(struct dma_buf_attach_sysfs_entry *sysfs_entry,
- struct dma_buf_attach_stats_attribute *attr, char *buf);
-};
-#define to_dma_buf_attach_stats_attr(x) container_of(x, struct
dma_buf_attach_stats_attribute, attr)
-
-static ssize_t dma_buf_attach_stat
[PATCH 1/1] drm: bridge: Mark deprecated operations in drm_bridge_funcs
drm_bridge_funcs includes several duplicated operations as atomic
variants has been added over time.
New bridge drivers shall use the atomic variants - mark the deprecated
operations to try to avoid usage in new bridge drivers.
Signed-off-by: Sam Ravnborg
Cc: Laurent Pinchart
Cc: Andrzej Hajda
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: Thomas Zimmermann
Cc: David Airlie
Cc: Daniel Vetter
---
include/drm/drm_bridge.h | 28 ++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 2195daa289d2..6805898d70f5 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -171,6 +171,11 @@ struct drm_bridge_funcs {
* signals) feeding it is still running when this callback is called.
*
* The @disable callback is optional.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall use &drm_bridge_funcs.atomic_disable.
*/
void (*disable)(struct drm_bridge *bridge);
@@ -190,6 +195,11 @@ struct drm_bridge_funcs {
* called.
*
* The @post_disable callback is optional.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall use &drm_bridge_funcs.atomic_post_disable.
*/
void (*post_disable)(struct drm_bridge *bridge);
@@ -213,11 +223,15 @@ struct drm_bridge_funcs {
* For atomic drivers the adjusted_mode is the mode stored in
* &drm_crtc_state.adjusted_mode.
*
-* NOTE:
-*
* If a need arises to store and access modes adjusted for other
* locations than the connection between the CRTC and the first bridge,
* the DRM framework will have to be extended with DRM bridge states.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall set their mode in &drm_bridge_funcs.atomic_enable
+* operation.
*/
void (*mode_set)(struct drm_bridge *bridge,
const struct drm_display_mode *mode,
@@ -239,6 +253,11 @@ struct drm_bridge_funcs {
* there is one) when this callback is called.
*
* The @pre_enable callback is optional.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall use &drm_bridge_funcs.atomic_pre_enable.
*/
void (*pre_enable)(struct drm_bridge *bridge);
@@ -259,6 +278,11 @@ struct drm_bridge_funcs {
* chain if there is one.
*
* The @enable callback is optional.
+*
+* NOTE:
+*
+* This is deprecated, do not use!
+* New drivers shall use &drm_bridge_funcs.atomic_enable.
*/
void (*enable)(struct drm_bridge *bridge);
--
2.30.2
Re: [PATCH 14/16] drm/i915/guc/slpc: Sysfs hooks for slpc
Hi Vinay, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on drm-tip/drm-tip] [cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm/drm-next v5.13 next-20210709] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Vinay-Belgaumkar/Enable-GuC-based-power-management-features/20210710-092520 base: git://anongit.freedesktop.org/drm/drm-tip drm-tip config: x86_64-randconfig-s021-20210709 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.3-341-g8af24329-dirty # https://github.com/0day-ci/linux/commit/8388422991b4e0e4da460328634a7ec1d278de6a git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Vinay-Belgaumkar/Enable-GuC-based-power-management-features/20210710-092520 git checkout 8388422991b4e0e4da460328634a7ec1d278de6a # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/i915/gt/intel_rps.c:1978:5: sparse: sparse: symbol >> 'intel_rps_get_req' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
[RFC PATCH] drm/i915/guc/slpc: intel_rps_read_punit_req() can be static
drivers/gpu/drm/i915/gt/intel_rps.c:1969:5: warning: symbol
'intel_rps_read_punit_req' was not declared. Should it be static?
drivers/gpu/drm/i915/gt/intel_rps.c:1978:5: warning: symbol 'intel_rps_get_req'
was not declared. Should it be static?
Reported-by: kernel test robot
Signed-off-by: kernel test robot
---
intel_rps.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index 88ffc5d90730a..a78af6b0babea 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1966,7 +1966,7 @@ u32 intel_rps_read_actual_frequency(struct intel_rps *rps)
return freq;
}
-u32 intel_rps_read_punit_req(struct intel_rps *rps)
+static u32 intel_rps_read_punit_req(struct intel_rps *rps)
{
struct intel_uncore *uncore = rps_to_uncore(rps);
@@ -1975,7 +1975,7 @@ u32 intel_rps_read_punit_req(struct intel_rps *rps)
return pureq;
}
-u32 intel_rps_get_req(struct intel_rps *rps, u32 pureq)
+static u32 intel_rps_get_req(struct intel_rps *rps, u32 pureq)
{
u32 req = pureq >> GEN9_SW_REQ_UNSLICE_RATIO_SHIFT;
[Bug 212469] plymouth animation freezes during shutdown
https://bugzilla.kernel.org/show_bug.cgi?id=212469 --- Comment #10 from Amir (amirg...@criptext.com) --- The problem introduced with Kernel 5.10 But then I installed the git build of plymouth and it was gone! So I assume the problem brought up by changes in kernel, and plymouth patched their app to resolve the issue. Your git build seems fairly outdated. Could you build directly from git and test again? -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 212469] plymouth animation freezes during shutdown
https://bugzilla.kernel.org/show_bug.cgi?id=212469 --- Comment #9 from Norbert (asteri...@gmx.de) --- With Linux 5.13.0-11-generic #11-Ubuntu SMP Tue Jun 29 06:57:28 UTC 2021 no change. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH] drm: bridge: nwl-dsi: Drop unused nwl_dsi_plat_clk_config
Hi Jagan,
On Sun, Jul 04, 2021 at 03:04:33PM +0530, Jagan Teki wrote:
> nwl_dsi_plat_clk_config structure added in below commit but not
> used anywhere in the driver.
>
> commit <44cfc6233447c> ("drm/bridge: Add NWL MIPI DSI host controller
> support")
>
> Drop it.
>
> Cc: Guido Günther
> Signed-off-by: Jagan Teki
Applied to drm-misc-next.
Sam
Re: [PATCH 06/12] drm/mgag200: Store values (not bits) in struct mgag200_pll_values
Hi Thomas,
On Mon, Jul 05, 2021 at 02:45:09PM +0200, Thomas Zimmermann wrote:
> The fields in struct mgag200_pll_values currently hold the bits of
> each register. Store the PLL values instead and let the PLL-update
> code figure out the bits for each register.
>
> Signed-off-by: Thomas Zimmermann
I gave up trying to follow the changes in this patch.
Also I was left with the impression that this was no win in readability
at it looks to me like changes with a high risk to introduce a
hard-to-find bug.
Your changelog did not justify why this change is a win, only what is
does. But whatever works better for you I guess..
Sam
> ---
> drivers/gpu/drm/mgag200/mgag200_mode.c | 153 +++--
> 1 file changed, 91 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 9f6fe7673674..7d6707bd6c27 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -123,7 +123,7 @@ static int mgag200_compute_pixpll_values_g200(struct
> mga_device *mdev, long cloc
> const int in_div_max = 6;
> const int feed_div_min = 7;
> const int feed_div_max = 127;
> - u8 testm, testn;
> + u8 testp, testm, testn;
> u8 n = 0, m = 0, p, s;
> long f_vco;
> long computed;
> @@ -141,10 +141,11 @@ static int mgag200_compute_pixpll_values_g200(struct
> mga_device *mdev, long cloc
> clock = p_clk_min >> 3;
>
> f_vco = clock;
> - for (p = 0;
> - p <= post_div_max && f_vco < p_clk_min;
> - p = (p << 1) + 1, f_vco <<= 1)
> + for (testp = 0;
> + testp <= post_div_max && f_vco < p_clk_min;
> + testp = (testp << 1) + 1, f_vco <<= 1)
> ;
> + p = testp + 1;
>
> delta = clock;
>
> @@ -157,12 +158,12 @@ static int mgag200_compute_pixpll_values_g200(struct
> mga_device *mdev, long cloc
> tmp_delta = computed - f_vco;
> if (tmp_delta < delta) {
> delta = tmp_delta;
> - m = testm;
> - n = testn;
> + m = testm + 1;
> + n = testn + 1;
> }
> }
> }
> - f_vco = ref_clk * (n + 1) / (m + 1);
> + f_vco = ref_clk * n / m;
> if (f_vco < 10)
> s = 0;
> else if (f_vco < 14)
> @@ -186,6 +187,7 @@ static int mgag200_compute_pixpll_values_g200(struct
> mga_device *mdev, long cloc
> static void mgag200_set_pixpll_g200(struct mga_device *mdev,
> const struct mgag200_pll_values *pixpllc)
> {
> + unsigned int pixpllcm, pixpllcn, pixpllcp, pixpllcs;
> u8 misc, xpixpllcm, xpixpllcn, xpixpllcp;
>
> misc = RREG8(MGA_MISC_IN);
> @@ -193,9 +195,14 @@ static void mgag200_set_pixpll_g200(struct mga_device
> *mdev,
> misc |= MGAREG_MISC_CLK_SEL_MGA_MSK;
> WREG8(MGA_MISC_OUT, misc);
>
> - xpixpllcm = pixpllc->m;
> - xpixpllcn = pixpllc->n;
> - xpixpllcp = pixpllc->p | (pixpllc->s << 3);
> + pixpllcm = pixpllc->m - 1;
> + pixpllcn = pixpllc->n - 1;
> + pixpllcp = pixpllc->p - 1;
> + pixpllcs = pixpllc->s;
> +
> + xpixpllcm = pixpllcm;
> + xpixpllcn = pixpllcn;
> + xpixpllcp = (pixpllcs << 3) | pixpllcp;
> WREG_DAC(MGA1064_PIX_PLLC_M, xpixpllcm);
> WREG_DAC(MGA1064_PIX_PLLC_N, xpixpllcn);
> WREG_DAC(MGA1064_PIX_PLLC_P, xpixpllcp);
> @@ -240,9 +247,9 @@ static int mgag200_compute_pixpll_values_g200se(struct
> mga_device *mdev, long cl
> tmpdelta = clock - computed;
> if (tmpdelta < delta) {
> delta = tmpdelta;
> - m = testm - 1;
> - n = testn - 1;
> - p = testp - 1;
> + m = testm;
> + n = testn;
> + p = testp;
> }
> }
> }
> @@ -280,22 +287,19 @@ static int mgag200_compute_pixpll_values_g200se(struct
> mga_device *mdev, long cl
>
> if (tmpdelta < delta) {
> delta = tmpdelta;
> - m = testm - 1;
> - n = testn - 1;
> - p = testp - 1;
> + m = testm;
> + n = testn;
> + p = testp;
>
Re: [PATCH 11/12] drm/mgag200: Introduce custom CRTC state
Hi Thomas,
On Mon, Jul 05, 2021 at 02:45:14PM +0200, Thomas Zimmermann wrote:
> The CRTC state in mgag200 will hold PLL values for modeset operations.
> Simple KMS helpers already support custom state for planes. Extend the
> helpers to support custom CRTC state as well.
This should be split in two patches - so the changes to
drm_simple_kms_helper can get more attention.
The patch as such looks fine so when split you can add my:
Acked-by: Sam Ravnborg
on the mgag200 parts.
And
Reviewed-by: Sam Ravnborg
on the drm_simple_kms_helper parts.
Sam
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_simple_kms_helper.c | 39 +++--
> drivers/gpu/drm/mgag200/mgag200_drv.h | 9 +
> drivers/gpu/drm/mgag200/mgag200_mode.c | 46 +
> include/drm/drm_simple_kms_helper.h | 27 +++
> 4 files changed, 118 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c
> b/drivers/gpu/drm/drm_simple_kms_helper.c
> index 735f4f34bcc4..72989ed1baba 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -145,6 +145,39 @@ static const struct drm_crtc_helper_funcs
> drm_simple_kms_crtc_helper_funcs = {
> .atomic_disable = drm_simple_kms_crtc_disable,
> };
>
> +static void drm_simple_kms_crtc_reset(struct drm_crtc *crtc)
> +{
> + struct drm_simple_display_pipe *pipe;
> +
> + pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
> + if (!pipe->funcs || !pipe->funcs->reset_crtc)
> + return drm_atomic_helper_crtc_reset(crtc);
> +
> + return pipe->funcs->reset_crtc(pipe);
> +}
> +
> +static struct drm_crtc_state *drm_simple_kms_crtc_duplicate_state(struct
> drm_crtc *crtc)
> +{
> + struct drm_simple_display_pipe *pipe;
> +
> + pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
> + if (!pipe->funcs || !pipe->funcs->duplicate_crtc_state)
> + return drm_atomic_helper_crtc_duplicate_state(crtc);
> +
> + return pipe->funcs->duplicate_crtc_state(pipe);
> +}
> +
> +static void drm_simple_kms_crtc_destroy_state(struct drm_crtc *crtc, struct
> drm_crtc_state *state)
> +{
> + struct drm_simple_display_pipe *pipe;
> +
> + pipe = container_of(crtc, struct drm_simple_display_pipe, crtc);
> + if (!pipe->funcs || !pipe->funcs->destroy_crtc_state)
> + drm_atomic_helper_crtc_destroy_state(crtc, state);
> + else
> + pipe->funcs->destroy_crtc_state(pipe, state);
> +}
> +
> static int drm_simple_kms_crtc_enable_vblank(struct drm_crtc *crtc)
> {
> struct drm_simple_display_pipe *pipe;
> @@ -168,12 +201,12 @@ static void drm_simple_kms_crtc_disable_vblank(struct
> drm_crtc *crtc)
> }
>
> static const struct drm_crtc_funcs drm_simple_kms_crtc_funcs = {
> - .reset = drm_atomic_helper_crtc_reset,
> + .reset = drm_simple_kms_crtc_reset,
> .destroy = drm_crtc_cleanup,
> .set_config = drm_atomic_helper_set_config,
> .page_flip = drm_atomic_helper_page_flip,
> - .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> - .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> + .atomic_duplicate_state = drm_simple_kms_crtc_duplicate_state,
> + .atomic_destroy_state = drm_simple_kms_crtc_destroy_state,
> .enable_vblank = drm_simple_kms_crtc_enable_vblank,
> .disable_vblank = drm_simple_kms_crtc_disable_vblank,
> };
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index fca3904fde0c..c813d6c15f70 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -127,6 +127,15 @@ struct mgag200_pll_values {
> unsigned int s;
> };
>
> +struct mgag200_crtc_state {
> + struct drm_crtc_state base;
> +};
> +
> +static inline struct mgag200_crtc_state *to_mgag200_crtc_state(struct
> drm_crtc_state *base)
> +{
> + return container_of(base, struct mgag200_crtc_state, base);
> +}
> +
> #define to_mga_connector(x) container_of(x, struct mga_connector, base)
>
> struct mga_i2c_chan {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 5da72ebd8a7f..6a5c419c6641 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1886,6 +1886,49 @@ mgag200_simple_display_pipe_update(struct
> drm_simple_display_pipe *pipe,
> mgag200_handle_damage(mdev, fb, &damage,
> &shadow_plane_state->map[0]);
> }
>
> +static struct drm_crtc_state *
> +mgag200_simple_display_pipe_duplicate_crtc_state(struct
> drm_simple_display_pipe *pipe)
> +{
> + struct drm_crtc *crtc = &pipe->crtc;
> + struct drm_crtc_state *crtc_state = crtc->state;
> + struct mgag200_crtc_state *new_mgag200_crtc_state;
> +
> + if (!crtc_state)
> + return NULL;
> +
> + new_mgag200_crtc_st
