* [PATCH 0/7] drm/mediatek: fix cursor issue and apply CMDQ in
@ 2019-11-26 6:29 Bibby Hsieh
2019-11-26 6:29 ` [PATCH 1/7] drm/mediatek: fix atomic_state reference counting Bibby Hsieh
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-26 6:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
Cc: drinkcat, Bibby Hsieh, srv_heupstream, linux-kernel, tfiga,
CK Hu, Thierry Reding, Philipp Zabel, linux-arm-kernel
The CMDQ (Command Queue) in MT8183 is used to help update all
relevant display controller registers with critical time limation.
This patch add cmdq interface in ddp_comp interface, let all
ddp_comp interface can support cpu/cmdq function at the same time.
These patches also can fixup cursor moving is not smooth
when heavy load in webgl.
This patch depends on ptach:
add drm support for MT8183
(https://patchwork.kernel.org/cover/11121519/)
support gce on mt8183 platform
(https://patchwork.kernel.org/cover/11255147)
drm/mediatek: Refactor plane init/check and support rotation
(https://pw-emeril.freedesktop.org/series/69015/)
drm/mediatek: Check return value of mtk_drm_ddp_comp_for_plane
(https://lore.kernel.org/patchwork/patch/1154517/)
Bibby Hsieh (7):
drm/mediatek: fix atomic_state reference counting
drm/mediatek: put "event" in critical section
drm/mediatek: use DRM core's atomic commit helper
drm/mediatek: handle events when enabling/disabling crtc
drm/mediatek: update cursors by using async atomic update
drm/mediatek: support CMDQ interface in ddp component
drm/mediatek: apply CMDQ control flow
drivers/gpu/drm/mediatek/mtk_disp_color.c | 7 +-
drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 65 +++---
drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 43 ++--
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 223 ++++++++++++++++++--
drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 4 +
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 151 +++++++++----
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 55 +++--
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 143 +++++++------
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 17 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 54 +++++
drivers/gpu/drm/mediatek/mtk_drm_plane.h | 2 +
11 files changed, 575 insertions(+), 189 deletions(-)
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/7] drm/mediatek: fix atomic_state reference counting
2019-11-26 6:29 [PATCH 0/7] drm/mediatek: fix cursor issue and apply CMDQ in Bibby Hsieh
@ 2019-11-26 6:29 ` Bibby Hsieh
2019-11-26 8:49 ` Daniel Vetter
2019-11-26 6:29 ` [PATCH 2/7] drm/mediatek: put "event" in critical section Bibby Hsieh
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-26 6:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
Cc: drinkcat, Bibby Hsieh, srv_heupstream, linux-kernel, tfiga,
CK Hu, Thierry Reding, Philipp Zabel, linux-arm-kernel
The DRM core takes care of all atomic state refcounting.
However, mediatek drm defers some work that accesses planes
and plane_states in drm_atomic_state, and must therefore
keep its own atomic state references until this work complete.
We take the atomic_state reference in atomic_fulsh() and ensure all the
information in atomic_state already was updated in hardware for
showing on screen and then schedules unreference_work to drop references
on atomic_state.
Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 11 +++-
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 79 +++++++++++++++++++++++++
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 9 +++
3 files changed, 97 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 29d0582e90e9..68b92adc96bb 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -7,7 +7,7 @@
#include <linux/pm_runtime.h>
#include <asm/barrier.h>
-
+#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_plane_helper.h>
#include <drm/drm_probe_helper.h>
@@ -47,6 +47,7 @@ struct mtk_drm_crtc {
struct mtk_disp_mutex *mutex;
unsigned int ddp_comp_nr;
struct mtk_ddp_comp **ddp_comp;
+ struct drm_crtc_state *old_crtc_state;
};
struct mtk_crtc_state {
@@ -362,6 +363,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
{
struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+ struct drm_atomic_state *atomic_state = mtk_crtc->old_crtc_state->state;
struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
unsigned int i;
@@ -399,6 +401,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
plane_state->pending.config = false;
}
mtk_crtc->pending_planes = false;
+ mtk_atomic_state_put_queue(atomic_state);
}
}
@@ -494,6 +497,7 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
{
+ struct drm_atomic_state *old_atomic_state = old_crtc_state->state;
struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
struct mtk_drm_private *priv = crtc->dev->dev_private;
unsigned int pending_planes = 0;
@@ -512,8 +516,11 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
pending_planes |= BIT(i);
}
}
- if (pending_planes)
+ if (pending_planes) {
mtk_crtc->pending_planes = true;
+ drm_atomic_state_get(old_atomic_state);
+ mtk_crtc->old_crtc_state = old_crtc_state;
+ }
if (crtc->state->color_mgmt_changed)
for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 6588dc6dd5e3..6c68283b6124 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -115,10 +115,85 @@ static int mtk_atomic_commit(struct drm_device *drm,
return 0;
}
+struct mtk_atomic_state {
+ struct drm_atomic_state base;
+ struct list_head list;
+};
+
+static inline struct mtk_atomic_state *to_mtk_state(struct drm_atomic_state *s)
+{
+ return container_of(s, struct mtk_atomic_state, base);
+}
+
+void mtk_atomic_state_put_queue(struct drm_atomic_state *state)
+{
+ struct drm_device *drm = state->dev;
+ struct mtk_drm_private *mtk_drm = drm->dev_private;
+ struct mtk_atomic_state *mtk_state = to_mtk_state(state);
+ unsigned long flags;
+
+ spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
+ list_add_tail(&mtk_state->list, &mtk_drm->unreference.list);
+ spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
+
+ schedule_work(&mtk_drm->unreference.work);
+}
+
+static void mtk_unreference_work(struct work_struct *work)
+{
+ struct mtk_drm_private *mtk_drm = container_of(work,
+ struct mtk_drm_private, unreference.work);
+ unsigned long flags;
+ struct mtk_atomic_state *state, *tmp;
+
+ /*
+ * framebuffers cannot be unreferenced in atomic context.
+ * Therefore, only hold the spinlock when iterating unreference_list,
+ * and drop it when doing the unreference.
+ */
+ spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
+ list_for_each_entry_safe(state, tmp, &mtk_drm->unreference.list, list) {
+ list_del(&state->list);
+ spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
+ drm_atomic_state_put(&state->base);
+ spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
+ }
+ spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
+}
+
+static struct drm_atomic_state *
+ mtk_drm_atomic_state_alloc(struct drm_device *dev)
+{
+ struct mtk_atomic_state *mtk_state;
+
+ mtk_state = kzalloc(sizeof(*mtk_state), GFP_KERNEL);
+ if (!mtk_state)
+ return NULL;
+
+ if (drm_atomic_state_init(dev, &mtk_state->base) < 0) {
+ kfree(mtk_state);
+ return NULL;
+ }
+
+ INIT_LIST_HEAD(&mtk_state->list);
+
+ return &mtk_state->base;
+}
+
+static void mtk_drm_atomic_state_free(struct drm_atomic_state *state)
+{
+ struct mtk_atomic_state *mtk_state = to_mtk_state(state);
+
+ drm_atomic_state_default_release(state);
+ kfree(mtk_state);
+}
+
static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
.fb_create = mtk_drm_mode_fb_create,
.atomic_check = drm_atomic_helper_check,
.atomic_commit = mtk_atomic_commit,
+ .atomic_state_alloc = mtk_drm_atomic_state_alloc,
+ .atomic_state_free = mtk_drm_atomic_state_free
};
static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
@@ -337,6 +412,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
drm_kms_helper_poll_init(drm);
drm_mode_config_reset(drm);
+ INIT_WORK(&private->unreference.work, mtk_unreference_work);
+ INIT_LIST_HEAD(&private->unreference.list);
+ spin_lock_init(&private->unreference.lock);
+
return 0;
err_unset_dma_parms:
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index b6a82728d563..c37d835cf949 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -55,6 +55,13 @@ struct mtk_drm_private {
struct drm_atomic_state *suspend_state;
+ struct {
+ struct work_struct work;
+ struct list_head list;
+ /* lock for unreference list */
+ spinlock_t lock;
+ } unreference;
+
bool dma_parms_allocated;
};
@@ -66,4 +73,6 @@ extern struct platform_driver mtk_dpi_driver;
extern struct platform_driver mtk_dsi_driver;
extern struct platform_driver mtk_mipi_tx_driver;
+void mtk_atomic_state_put_queue(struct drm_atomic_state *state);
+
#endif /* MTK_DRM_DRV_H */
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/7] drm/mediatek: put "event" in critical section
2019-11-26 6:29 [PATCH 0/7] drm/mediatek: fix cursor issue and apply CMDQ in Bibby Hsieh
2019-11-26 6:29 ` [PATCH 1/7] drm/mediatek: fix atomic_state reference counting Bibby Hsieh
@ 2019-11-26 6:29 ` Bibby Hsieh
2019-11-26 6:29 ` [PATCH 3/7] drm/mediatek: use DRM core's atomic commit helper Bibby Hsieh
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-26 6:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
Cc: drinkcat, Bibby Hsieh, srv_heupstream, linux-kernel, tfiga,
CK Hu, Thierry Reding, Philipp Zabel, linux-arm-kernel
The state->base.event variable would be access by thread context
in mtk_drm_crtc_atomic_begin() or by interrupt context in
mtk_drm_crtc_finish_page_flip(), so each part should be a critical
section. Fix it.
Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 68b92adc96bb..e38506d7a4e8 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -486,12 +486,15 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
if (mtk_crtc->event && state->base.event)
DRM_ERROR("new event while there is still a pending event\n");
+ spin_lock_irq(&crtc->dev->event_lock);
if (state->base.event) {
state->base.event->pipe = drm_crtc_index(crtc);
WARN_ON(drm_crtc_vblank_get(crtc) != 0);
+ WARN_ON(mtk_crtc->event);
mtk_crtc->event = state->base.event;
state->base.event = NULL;
}
+ spin_unlock_irq(&crtc->dev->event_lock);
}
static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/7] drm/mediatek: use DRM core's atomic commit helper
2019-11-26 6:29 [PATCH 0/7] drm/mediatek: fix cursor issue and apply CMDQ in Bibby Hsieh
2019-11-26 6:29 ` [PATCH 1/7] drm/mediatek: fix atomic_state reference counting Bibby Hsieh
2019-11-26 6:29 ` [PATCH 2/7] drm/mediatek: put "event" in critical section Bibby Hsieh
@ 2019-11-26 6:29 ` Bibby Hsieh
2019-11-26 6:29 ` [PATCH 4/7] drm/mediatek: handle events when enabling/disabling crtc Bibby Hsieh
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-26 6:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
Cc: drinkcat, Bibby Hsieh, srv_heupstream, linux-kernel, tfiga,
CK Hu, Thierry Reding, Philipp Zabel, linux-arm-kernel
The DRM core atomic helper now supports asynchronous commits natively.
The custom drm implementation isn't needed anymore, remove it.
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 92 +++++---------------------
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 7 --
2 files changed, 16 insertions(+), 83 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 6c68283b6124..97949dee2af0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -36,84 +36,25 @@
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
-static void mtk_atomic_schedule(struct mtk_drm_private *private,
- struct drm_atomic_state *state)
+static void mtk_drm_atomic_helper_commit_tail_rpm
+ (struct drm_atomic_state *old_state)
{
- private->commit.state = state;
- schedule_work(&private->commit.work);
-}
-
-static void mtk_atomic_complete(struct mtk_drm_private *private,
- struct drm_atomic_state *state)
-{
- struct drm_device *drm = private->drm;
+ struct drm_device *dev = old_state->dev;
- drm_atomic_helper_wait_for_fences(drm, state, false);
-
- /*
- * Mediatek drm supports runtime PM, so plane registers cannot be
- * written when their crtc is disabled.
- *
- * The comment for drm_atomic_helper_commit states:
- * For drivers supporting runtime PM the recommended sequence is
- *
- * drm_atomic_helper_commit_modeset_disables(dev, state);
- * drm_atomic_helper_commit_modeset_enables(dev, state);
- * drm_atomic_helper_commit_planes(dev, state,
- * DRM_PLANE_COMMIT_ACTIVE_ONLY);
- *
- * See the kerneldoc entries for these three functions for more details.
- */
- drm_atomic_helper_commit_modeset_disables(drm, state);
- drm_atomic_helper_commit_modeset_enables(drm, state);
- drm_atomic_helper_commit_planes(drm, state,
+ drm_atomic_helper_wait_for_fences(dev, old_state, false);
+ drm_atomic_helper_commit_modeset_disables(dev, old_state);
+ drm_atomic_helper_commit_modeset_enables(dev, old_state);
+ drm_atomic_helper_commit_planes(dev, old_state,
DRM_PLANE_COMMIT_ACTIVE_ONLY);
-
- drm_atomic_helper_wait_for_vblanks(drm, state);
-
- drm_atomic_helper_cleanup_planes(drm, state);
- drm_atomic_state_put(state);
+ drm_atomic_helper_fake_vblank(old_state);
+ drm_atomic_helper_commit_hw_done(old_state);
+ drm_atomic_helper_wait_for_vblanks(dev, old_state);
+ drm_atomic_helper_cleanup_planes(dev, old_state);
}
-static void mtk_atomic_work(struct work_struct *work)
-{
- struct mtk_drm_private *private = container_of(work,
- struct mtk_drm_private, commit.work);
-
- mtk_atomic_complete(private, private->commit.state);
-}
-
-static int mtk_atomic_commit(struct drm_device *drm,
- struct drm_atomic_state *state,
- bool async)
-{
- struct mtk_drm_private *private = drm->dev_private;
- int ret;
-
- ret = drm_atomic_helper_prepare_planes(drm, state);
- if (ret)
- return ret;
-
- mutex_lock(&private->commit.lock);
- flush_work(&private->commit.work);
-
- ret = drm_atomic_helper_swap_state(state, true);
- if (ret) {
- mutex_unlock(&private->commit.lock);
- drm_atomic_helper_cleanup_planes(drm, state);
- return ret;
- }
-
- drm_atomic_state_get(state);
- if (async)
- mtk_atomic_schedule(private, state);
- else
- mtk_atomic_complete(private, state);
-
- mutex_unlock(&private->commit.lock);
-
- return 0;
-}
+static const struct drm_mode_config_helper_funcs mtk_drm_mode_config_helpers = {
+ .atomic_commit_tail = mtk_drm_atomic_helper_commit_tail_rpm,
+};
struct mtk_atomic_state {
struct drm_atomic_state base;
@@ -191,7 +132,7 @@ static void mtk_drm_atomic_state_free(struct drm_atomic_state *state)
static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
.fb_create = mtk_drm_mode_fb_create,
.atomic_check = drm_atomic_helper_check,
- .atomic_commit = mtk_atomic_commit,
+ .atomic_commit = drm_atomic_helper_commit,
.atomic_state_alloc = mtk_drm_atomic_state_alloc,
.atomic_state_free = mtk_drm_atomic_state_free
};
@@ -340,6 +281,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
drm->mode_config.max_width = 4096;
drm->mode_config.max_height = 4096;
drm->mode_config.funcs = &mtk_drm_mode_config_funcs;
+ drm->mode_config.helper_private = &mtk_drm_mode_config_helpers;
ret = component_bind_all(drm->dev, drm);
if (ret)
@@ -619,8 +561,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
if (!private)
return -ENOMEM;
- mutex_init(&private->commit.lock);
- INIT_WORK(&private->commit.work, mtk_atomic_work);
private->data = of_device_get_match_data(dev);
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index c37d835cf949..3c42383dedb0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -46,13 +46,6 @@ struct mtk_drm_private {
struct device_node *comp_node[DDP_COMPONENT_ID_MAX];
struct mtk_ddp_comp *ddp_comp[DDP_COMPONENT_ID_MAX];
const struct mtk_mmsys_driver_data *data;
-
- struct {
- struct drm_atomic_state *state;
- struct work_struct work;
- struct mutex lock;
- } commit;
-
struct drm_atomic_state *suspend_state;
struct {
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/7] drm/mediatek: handle events when enabling/disabling crtc
2019-11-26 6:29 [PATCH 0/7] drm/mediatek: fix cursor issue and apply CMDQ in Bibby Hsieh
` (2 preceding siblings ...)
2019-11-26 6:29 ` [PATCH 3/7] drm/mediatek: use DRM core's atomic commit helper Bibby Hsieh
@ 2019-11-26 6:29 ` Bibby Hsieh
2019-11-26 6:29 ` [PATCH 5/7] drm/mediatek: update cursors by using async atomic update Bibby Hsieh
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-26 6:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
Cc: drinkcat, Bibby Hsieh, srv_heupstream, linux-kernel, tfiga,
CK Hu, Thierry Reding, Philipp Zabel, linux-arm-kernel
The driver currently handles vblank events only when updating planes on
an already enabled CRTC. The atomic update API however allows requesting
an event when enabling or disabling a CRTC. This currently leads to
event objects being leaked in the kernel and to events not being sent
out. Fix it.
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index e38506d7a4e8..408a6d6a15ba 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -335,6 +335,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
{
struct drm_device *drm = mtk_crtc->base.dev;
+ struct drm_crtc *crtc = &mtk_crtc->base;
int i;
DRM_DEBUG_DRIVER("%s\n", __func__);
@@ -358,6 +359,13 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
mtk_disp_mutex_unprepare(mtk_crtc->mutex);
pm_runtime_put(drm->dev);
+
+ if (crtc->state->event && !crtc->state->active) {
+ spin_lock_irq(&crtc->dev->event_lock);
+ drm_crtc_send_vblank_event(crtc, crtc->state->event);
+ crtc->state->event = NULL;
+ spin_unlock_irq(&crtc->dev->event_lock);
+ }
}
static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/7] drm/mediatek: update cursors by using async atomic update
2019-11-26 6:29 [PATCH 0/7] drm/mediatek: fix cursor issue and apply CMDQ in Bibby Hsieh
` (3 preceding siblings ...)
2019-11-26 6:29 ` [PATCH 4/7] drm/mediatek: handle events when enabling/disabling crtc Bibby Hsieh
@ 2019-11-26 6:29 ` Bibby Hsieh
2019-11-26 6:29 ` [PATCH 6/7] drm/mediatek: support CMDQ interface in ddp component Bibby Hsieh
2019-11-26 6:29 ` [PATCH 7/7] drm/mediatek: apply CMDQ control flow Bibby Hsieh
6 siblings, 0 replies; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-26 6:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
Cc: drinkcat, Bibby Hsieh, srv_heupstream, linux-kernel, tfiga,
CK Hu, Thierry Reding, Philipp Zabel, linux-arm-kernel
Support to async updates of cursors by using the new atomic
interface for that.
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 41 ++++++++++++++++++-
drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 3 ++
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 ++
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 3 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 50 ++++++++++++++++++++++++
drivers/gpu/drm/mediatek/mtk_drm_plane.h | 2 +
6 files changed, 100 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 408a6d6a15ba..c457591a176e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -41,7 +41,7 @@ struct mtk_drm_crtc {
struct drm_plane *planes;
unsigned int layer_nr;
bool pending_planes;
-
+ bool cursor_update;
void __iomem *config_regs;
const struct mtk_mmsys_reg_data *mmsys_reg_data;
struct mtk_disp_mutex *mutex;
@@ -409,7 +409,9 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
plane_state->pending.config = false;
}
mtk_crtc->pending_planes = false;
- mtk_atomic_state_put_queue(atomic_state);
+ if (!mtk_crtc->cursor_update)
+ mtk_atomic_state_put_queue(atomic_state);
+ mtk_crtc->cursor_update = false;
}
}
@@ -425,6 +427,41 @@ int mtk_drm_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane *plane,
return 0;
}
+void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
+ struct drm_plane_state *new_state)
+{
+ struct mtk_drm_private *priv = crtc->dev->dev_private;
+ struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+ const struct drm_plane_helper_funcs *plane_helper_funcs =
+ plane->helper_private;
+ int i;
+
+ if (!mtk_crtc->enabled)
+ return;
+
+ mutex_lock(&priv->hw_lock);
+ plane_helper_funcs->atomic_update(plane, new_state);
+
+ for (i = 0; i < mtk_crtc->layer_nr; i++) {
+ struct drm_plane *plane = &mtk_crtc->planes[i];
+ struct mtk_plane_state *plane_state;
+
+ plane_state = to_mtk_plane_state(plane->state);
+ if (plane_state->pending.cursor_dirty) {
+ plane_state->pending.config = true;
+ plane_state->pending.cursor_update = false;
+ plane_state->pending.cursor_dirty = false;
+ }
+ }
+ mtk_crtc->pending_planes = true;
+ if (priv->data->shadow_register) {
+ mtk_disp_mutex_acquire(mtk_crtc->mutex);
+ mtk_crtc_ddp_config(crtc);
+ mtk_disp_mutex_release(mtk_crtc->mutex);
+ }
+ mutex_unlock(&priv->hw_lock);
+}
+
static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_crtc_state *old_state)
{
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
index 6afe1c19557a..42a3f650f564 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
@@ -22,4 +22,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
int mtk_drm_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane *plane,
struct mtk_plane_state *state);
+void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
+ struct drm_plane_state *plane_state);
+
#endif /* MTK_DRM_CRTC_H */
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 97949dee2af0..3884a89a07e6 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -40,12 +40,15 @@ static void mtk_drm_atomic_helper_commit_tail_rpm
(struct drm_atomic_state *old_state)
{
struct drm_device *dev = old_state->dev;
+ struct mtk_drm_private *private = dev->dev_private;
drm_atomic_helper_wait_for_fences(dev, old_state, false);
+ mutex_lock(&private->hw_lock);
drm_atomic_helper_commit_modeset_disables(dev, old_state);
drm_atomic_helper_commit_modeset_enables(dev, old_state);
drm_atomic_helper_commit_planes(dev, old_state,
DRM_PLANE_COMMIT_ACTIVE_ONLY);
+ mutex_unlock(&private->hw_lock);
drm_atomic_helper_fake_vblank(old_state);
drm_atomic_helper_commit_hw_done(old_state);
drm_atomic_helper_wait_for_vblanks(dev, old_state);
@@ -357,6 +360,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
INIT_WORK(&private->unreference.work, mtk_unreference_work);
INIT_LIST_HEAD(&private->unreference.list);
spin_lock_init(&private->unreference.lock);
+ mutex_init(&private->hw_lock);
return 0;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 3c42383dedb0..043bd2d210f2 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -54,7 +54,8 @@ struct mtk_drm_private {
/* lock for unreference list */
spinlock_t lock;
} unreference;
-
+ /* lock for display hw access */
+ struct mutex hw_lock;
bool dma_parms_allocated;
};
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index cd7c97eb7ee6..d7a8853d94a1 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -7,6 +7,7 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_fourcc.h>
+#include <drm/drm_atomic_uapi.h>
#include <drm/drm_plane_helper.h>
#include <drm/drm_gem_framebuffer_helper.h>
@@ -70,6 +71,50 @@ static void mtk_drm_plane_destroy_state(struct drm_plane *plane,
kfree(to_mtk_plane_state(state));
}
+static int mtk_plane_atomic_async_check(struct drm_plane *plane,
+ struct drm_plane_state *state)
+{
+ struct drm_crtc_state *crtc_state;
+
+ if (plane != state->crtc->cursor)
+ return -EINVAL;
+
+ if (!plane->state)
+ return -EINVAL;
+
+ if (!plane->state->fb)
+ return -EINVAL;
+
+ if (state->state)
+ crtc_state = drm_atomic_get_existing_crtc_state(state->state,
+ state->crtc);
+ else /* Special case for asynchronous cursor updates. */
+ crtc_state = state->crtc->state;
+
+ return drm_atomic_helper_check_plane_state(plane->state, crtc_state,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
+}
+
+static void mtk_plane_atomic_async_update(struct drm_plane *plane,
+ struct drm_plane_state *new_state)
+{
+ struct mtk_plane_state *state = to_mtk_plane_state(plane->state);
+
+ plane->state->crtc_x = new_state->crtc_x;
+ plane->state->crtc_y = new_state->crtc_y;
+ plane->state->crtc_h = new_state->crtc_h;
+ plane->state->crtc_w = new_state->crtc_w;
+ plane->state->src_x = new_state->src_x;
+ plane->state->src_y = new_state->src_y;
+ plane->state->src_h = new_state->src_h;
+ plane->state->src_w = new_state->src_w;
+ state->pending.cursor_update = true;
+
+ mtk_drm_crtc_cursor_update(new_state->crtc, plane, new_state);
+}
+
static const struct drm_plane_funcs mtk_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -141,6 +186,9 @@ static void mtk_plane_atomic_update(struct drm_plane *plane,
state->pending.rotation = plane->state->rotation;
wmb(); /* Make sure the above parameters are set before update */
state->pending.dirty = true;
+
+ if (state->pending.cursor_update)
+ state->pending.cursor_dirty = true;
}
static void mtk_plane_atomic_disable(struct drm_plane *plane,
@@ -158,6 +206,8 @@ static const struct drm_plane_helper_funcs mtk_plane_helper_funcs = {
.atomic_check = mtk_plane_atomic_check,
.atomic_update = mtk_plane_atomic_update,
.atomic_disable = mtk_plane_atomic_disable,
+ .atomic_async_update = mtk_plane_atomic_async_update,
+ .atomic_async_check = mtk_plane_atomic_async_check,
};
int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
index 760885e35b27..113a10344805 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h
@@ -22,6 +22,8 @@ struct mtk_plane_pending_state {
unsigned int height;
unsigned int rotation;
bool dirty;
+ bool cursor_dirty;
+ bool cursor_update;
};
struct mtk_plane_state {
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 6/7] drm/mediatek: support CMDQ interface in ddp component
2019-11-26 6:29 [PATCH 0/7] drm/mediatek: fix cursor issue and apply CMDQ in Bibby Hsieh
` (4 preceding siblings ...)
2019-11-26 6:29 ` [PATCH 5/7] drm/mediatek: update cursors by using async atomic update Bibby Hsieh
@ 2019-11-26 6:29 ` Bibby Hsieh
2019-11-26 6:29 ` [PATCH 7/7] drm/mediatek: apply CMDQ control flow Bibby Hsieh
6 siblings, 0 replies; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-26 6:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
Cc: drinkcat, Bibby Hsieh, srv_heupstream, Yongqiang Niu,
linux-kernel, tfiga, CK Hu, Thierry Reding, Philipp Zabel,
linux-arm-kernel
The CMDQ (Command Queue) in MT8183 is used to help
update all relevant display controller registers
with critical time limation.
This patch add cmdq interface in ddp_comp interface,
let all ddp_comp interface can support cpu/cmdq function
at the same time.
Signed-off-by: YT Shen <yt.shen@mediatek.com>
Signed-off-by: CK Hu <ck.hu@mediatek.com>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_disp_color.c | 7 +-
drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 65 ++++++-----
drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 43 ++++---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 11 +-
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 120 ++++++++++++++------
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 55 ++++++---
6 files changed, 190 insertions(+), 111 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c
index 59de2a46aa49..6fb0d6983a4a 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_color.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c
@@ -9,6 +9,7 @@
#include <linux/of_device.h>
#include <linux/of_irq.h>
#include <linux/platform_device.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
#include "mtk_drm_crtc.h"
#include "mtk_drm_ddp_comp.h"
@@ -45,12 +46,12 @@ static inline struct mtk_disp_color *comp_to_color(struct mtk_ddp_comp *comp)
static void mtk_color_config(struct mtk_ddp_comp *comp, unsigned int w,
unsigned int h, unsigned int vrefresh,
- unsigned int bpc)
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
struct mtk_disp_color *color = comp_to_color(comp);
- writel(w, comp->regs + DISP_COLOR_WIDTH(color));
- writel(h, comp->regs + DISP_COLOR_HEIGHT(color));
+ mtk_ddp_write(cmdq_pkt, w, comp, DISP_COLOR_WIDTH(color));
+ mtk_ddp_write(cmdq_pkt, h, comp, DISP_COLOR_HEIGHT(color));
}
static void mtk_color_start(struct mtk_ddp_comp *comp)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 722a5adb79dc..0732cd3ee4c8 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -12,6 +12,7 @@
#include <linux/of_irq.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
#include "mtk_drm_crtc.h"
#include "mtk_drm_ddp_comp.h"
@@ -125,14 +126,15 @@ static void mtk_ovl_stop(struct mtk_ddp_comp *comp)
static void mtk_ovl_config(struct mtk_ddp_comp *comp, unsigned int w,
unsigned int h, unsigned int vrefresh,
- unsigned int bpc)
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
if (w != 0 && h != 0)
- writel_relaxed(h << 16 | w, comp->regs + DISP_REG_OVL_ROI_SIZE);
- writel_relaxed(0x0, comp->regs + DISP_REG_OVL_ROI_BGCLR);
+ mtk_ddp_write_relaxed(cmdq_pkt, h << 16 | w, comp,
+ DISP_REG_OVL_ROI_SIZE);
+ mtk_ddp_write_relaxed(cmdq_pkt, 0x0, comp, DISP_REG_OVL_ROI_BGCLR);
- writel(0x1, comp->regs + DISP_REG_OVL_RST);
- writel(0x0, comp->regs + DISP_REG_OVL_RST);
+ mtk_ddp_write(cmdq_pkt, 0x1, comp, DISP_REG_OVL_RST);
+ mtk_ddp_write(cmdq_pkt, 0x0, comp, DISP_REG_OVL_RST);
}
static unsigned int mtk_ovl_layer_nr(struct mtk_ddp_comp *comp)
@@ -176,16 +178,16 @@ static int mtk_ovl_layer_check(struct mtk_ddp_comp *comp, unsigned int idx,
return 0;
}
-static void mtk_ovl_layer_on(struct mtk_ddp_comp *comp, unsigned int idx)
+static void mtk_ovl_layer_on(struct mtk_ddp_comp *comp, unsigned int idx,
+ struct cmdq_pkt *cmdq_pkt)
{
- unsigned int reg;
unsigned int gmc_thrshd_l;
unsigned int gmc_thrshd_h;
unsigned int gmc_value;
struct mtk_disp_ovl *ovl = comp_to_ovl(comp);
- writel(0x1, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx));
-
+ mtk_ddp_write(cmdq_pkt, 0x1, comp,
+ DISP_REG_OVL_RDMA_CTRL(idx));
gmc_thrshd_l = GMC_THRESHOLD_LOW >>
(GMC_THRESHOLD_BITS - ovl->data->gmc_bits);
gmc_thrshd_h = GMC_THRESHOLD_HIGH >>
@@ -195,22 +197,19 @@ static void mtk_ovl_layer_on(struct mtk_ddp_comp *comp, unsigned int idx)
else
gmc_value = gmc_thrshd_l | gmc_thrshd_l << 8 |
gmc_thrshd_h << 16 | gmc_thrshd_h << 24;
- writel(gmc_value, comp->regs + DISP_REG_OVL_RDMA_GMC(idx));
-
- reg = readl(comp->regs + DISP_REG_OVL_SRC_CON);
- reg = reg | BIT(idx);
- writel(reg, comp->regs + DISP_REG_OVL_SRC_CON);
+ mtk_ddp_write(cmdq_pkt, gmc_value,
+ comp, DISP_REG_OVL_RDMA_GMC(idx));
+ mtk_ddp_write_mask(cmdq_pkt, BIT(idx), comp,
+ DISP_REG_OVL_SRC_CON, BIT(idx));
}
-static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx)
+static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx,
+ struct cmdq_pkt *cmdq_pkt)
{
- unsigned int reg;
-
- reg = readl(comp->regs + DISP_REG_OVL_SRC_CON);
- reg = reg & ~BIT(idx);
- writel(reg, comp->regs + DISP_REG_OVL_SRC_CON);
-
- writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx));
+ mtk_ddp_write_mask(cmdq_pkt, 0, comp,
+ DISP_REG_OVL_SRC_CON, BIT(idx));
+ mtk_ddp_write(cmdq_pkt, 0, comp,
+ DISP_REG_OVL_RDMA_CTRL(idx));
}
static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt)
@@ -250,7 +249,8 @@ static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt)
}
static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
- struct mtk_plane_state *state)
+ struct mtk_plane_state *state,
+ struct cmdq_pkt *cmdq_pkt)
{
struct mtk_disp_ovl *ovl = comp_to_ovl(comp);
struct mtk_plane_pending_state *pending = &state->pending;
@@ -262,7 +262,7 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
unsigned int con;
if (!pending->enable)
- mtk_ovl_layer_off(comp, idx);
+ mtk_ovl_layer_off(comp, idx, cmdq_pkt);
con = ovl_fmt_convert(ovl, fmt);
if (idx != 0)
@@ -278,14 +278,19 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
addr += pending->pitch - 1;
}
- writel_relaxed(con, comp->regs + DISP_REG_OVL_CON(idx));
- writel_relaxed(pitch, comp->regs + DISP_REG_OVL_PITCH(idx));
- writel_relaxed(src_size, comp->regs + DISP_REG_OVL_SRC_SIZE(idx));
- writel_relaxed(offset, comp->regs + DISP_REG_OVL_OFFSET(idx));
- writel_relaxed(addr, comp->regs + DISP_REG_OVL_ADDR(ovl, idx));
+ mtk_ddp_write_relaxed(cmdq_pkt, con, comp,
+ DISP_REG_OVL_CON(idx));
+ mtk_ddp_write_relaxed(cmdq_pkt, pitch, comp,
+ DISP_REG_OVL_PITCH(idx));
+ mtk_ddp_write_relaxed(cmdq_pkt, src_size, comp,
+ DISP_REG_OVL_SRC_SIZE(idx));
+ mtk_ddp_write_relaxed(cmdq_pkt, offset, comp,
+ DISP_REG_OVL_OFFSET(idx));
+ mtk_ddp_write_relaxed(cmdq_pkt, addr, comp,
+ DISP_REG_OVL_ADDR(ovl, idx));
if (pending->enable)
- mtk_ovl_layer_on(comp, idx);
+ mtk_ovl_layer_on(comp, idx, cmdq_pkt);
}
static void mtk_ovl_bgclr_in_on(struct mtk_ddp_comp *comp)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
index 15cc4ae84aee..c1abde3743bf 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
@@ -10,6 +10,7 @@
#include <linux/of_irq.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
#include "mtk_drm_crtc.h"
#include "mtk_drm_ddp_comp.h"
@@ -127,15 +128,17 @@ static void mtk_rdma_stop(struct mtk_ddp_comp *comp)
static void mtk_rdma_config(struct mtk_ddp_comp *comp, unsigned int width,
unsigned int height, unsigned int vrefresh,
- unsigned int bpc)
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
unsigned int threshold;
unsigned int reg;
struct mtk_disp_rdma *rdma = comp_to_rdma(comp);
u32 rdma_fifo_size;
- rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_0, 0xfff, width);
- rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_1, 0xfffff, height);
+ mtk_ddp_write_mask(cmdq_pkt, width, comp,
+ DISP_REG_RDMA_SIZE_CON_0, 0xfff);
+ mtk_ddp_write_mask(cmdq_pkt, height, comp,
+ DISP_REG_RDMA_SIZE_CON_1, 0xfffff);
if (rdma->fifo_size)
rdma_fifo_size = rdma->fifo_size;
@@ -152,7 +155,7 @@ static void mtk_rdma_config(struct mtk_ddp_comp *comp, unsigned int width,
reg = RDMA_FIFO_UNDERFLOW_EN |
RDMA_FIFO_PSEUDO_SIZE(rdma_fifo_size) |
RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold);
- writel(reg, comp->regs + DISP_REG_RDMA_FIFO_CON);
+ mtk_ddp_write(cmdq_pkt, reg, comp, DISP_REG_RDMA_FIFO_CON);
}
static unsigned int rdma_fmt_convert(struct mtk_disp_rdma *rdma,
@@ -198,7 +201,8 @@ static unsigned int mtk_rdma_layer_nr(struct mtk_ddp_comp *comp)
}
static void mtk_rdma_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
- struct mtk_plane_state *state)
+ struct mtk_plane_state *state,
+ struct cmdq_pkt *cmdq_pkt)
{
struct mtk_disp_rdma *rdma = comp_to_rdma(comp);
struct mtk_plane_pending_state *pending = &state->pending;
@@ -208,24 +212,27 @@ static void mtk_rdma_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
unsigned int con;
con = rdma_fmt_convert(rdma, fmt);
- writel_relaxed(con, comp->regs + DISP_RDMA_MEM_CON);
+ mtk_ddp_write_relaxed(cmdq_pkt, con, comp, DISP_RDMA_MEM_CON);
if (fmt == DRM_FORMAT_UYVY || fmt == DRM_FORMAT_YUYV) {
- rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_0,
- RDMA_MATRIX_ENABLE, RDMA_MATRIX_ENABLE);
- rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_0,
- RDMA_MATRIX_INT_MTX_SEL,
- RDMA_MATRIX_INT_MTX_BT601_to_RGB);
+ mtk_ddp_write_mask(cmdq_pkt, RDMA_MATRIX_ENABLE, comp,
+ DISP_REG_RDMA_SIZE_CON_0,
+ RDMA_MATRIX_ENABLE);
+ mtk_ddp_write_mask(cmdq_pkt, RDMA_MATRIX_INT_MTX_BT601_to_RGB,
+ comp, DISP_REG_RDMA_SIZE_CON_0,
+ RDMA_MATRIX_INT_MTX_SEL);
} else {
- rdma_update_bits(comp, DISP_REG_RDMA_SIZE_CON_0,
- RDMA_MATRIX_ENABLE, 0);
+ mtk_ddp_write_mask(cmdq_pkt, 0, comp,
+ DISP_REG_RDMA_SIZE_CON_0,
+ RDMA_MATRIX_ENABLE);
}
+ mtk_ddp_write_relaxed(cmdq_pkt, addr, comp, DISP_RDMA_MEM_START_ADDR);
+ mtk_ddp_write_relaxed(cmdq_pkt, pitch, comp, DISP_RDMA_MEM_SRC_PITCH);
+ mtk_ddp_write(cmdq_pkt, RDMA_MEM_GMC, comp,
+ DISP_RDMA_MEM_GMC_SETTING_0);
+ mtk_ddp_write_mask(cmdq_pkt, RDMA_MODE_MEMORY, comp,
+ DISP_REG_RDMA_GLOBAL_CON, RDMA_MODE_MEMORY);
- writel_relaxed(addr, comp->regs + DISP_RDMA_MEM_START_ADDR);
- writel_relaxed(pitch, comp->regs + DISP_RDMA_MEM_SRC_PITCH);
- writel(RDMA_MEM_GMC, comp->regs + DISP_RDMA_MEM_GMC_SETTING_0);
- rdma_update_bits(comp, DISP_REG_RDMA_GLOBAL_CON,
- RDMA_MODE_MEMORY, RDMA_MODE_MEMORY);
}
static const struct mtk_ddp_comp_funcs mtk_disp_rdma_funcs = {
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index c457591a176e..3935fac624f6 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -305,7 +305,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
if (prev == DDP_COMPONENT_OVL0)
mtk_ddp_comp_bgclr_in_on(comp);
- mtk_ddp_comp_config(comp, width, height, vrefresh, bpc);
+ mtk_ddp_comp_config(comp, width, height, vrefresh, bpc, NULL);
mtk_ddp_comp_start(comp);
}
@@ -320,7 +320,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
comp = mtk_drm_ddp_comp_for_plane(crtc, plane, &local_layer);
if (comp)
mtk_ddp_comp_layer_config(comp, local_layer,
- plane_state);
+ plane_state, NULL);
}
return 0;
@@ -385,7 +385,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
if (state->pending_config) {
mtk_ddp_comp_config(comp, state->pending_width,
state->pending_height,
- state->pending_vrefresh, 0);
+ state->pending_vrefresh, 0, NULL);
state->pending_config = false;
}
@@ -405,7 +405,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
if (comp)
mtk_ddp_comp_layer_config(comp, local_layer,
- plane_state);
+ plane_state, NULL);
plane_state->pending.config = false;
}
mtk_crtc->pending_planes = false;
@@ -571,7 +571,8 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
}
if (crtc->state->color_mgmt_changed)
for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
- mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
+ mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i],
+ crtc->state, NULL);
if (priv->data->shadow_register) {
mtk_disp_mutex_acquire(mtk_crtc->mutex);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 3407d38aff8f..6d0f349ddf82 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -12,7 +12,8 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
-
+#include <drm/drmP.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
#include "mtk_drm_drv.h"
#include "mtk_drm_plane.h"
#include "mtk_drm_ddp_comp.h"
@@ -76,36 +77,82 @@
#define DITHER_ADD_LSHIFT_G(x) (((x) & 0x7) << 4)
#define DITHER_ADD_RSHIFT_G(x) (((x) & 0x7) << 0)
+void mtk_ddp_write(struct cmdq_pkt *cmdq_pkt, unsigned int value,
+ struct mtk_ddp_comp *comp, unsigned int offset)
+{
+ if (cmdq_pkt)
+#ifdef CONFIG_MTK_CMDQ
+ cmdq_pkt_write(cmdq_pkt, comp->subsys,
+ comp->regs_pa + offset, value);
+#endif
+ else
+ writel(value, comp->regs + offset);
+}
+
+void mtk_ddp_write_relaxed(struct cmdq_pkt *cmdq_pkt, unsigned int value,
+ struct mtk_ddp_comp *comp,
+ unsigned int offset)
+{
+ if (cmdq_pkt)
+#ifdef CONFIG_MTK_CMDQ
+ cmdq_pkt_write(cmdq_pkt, comp->subsys,
+ comp->regs_pa + offset, value);
+#endif
+ else
+ writel_relaxed(value, comp->regs + offset);
+}
+
+void mtk_ddp_write_mask(struct cmdq_pkt *cmdq_pkt,
+ unsigned int value,
+ struct mtk_ddp_comp *comp,
+ unsigned int offset,
+ unsigned int mask)
+{
+ if (cmdq_pkt) {
+#ifdef CONFIG_MTK_CMDQ
+ cmdq_pkt_write_mask(cmdq_pkt, comp->subsys,
+ comp->regs_pa + offset, value, mask);
+#endif
+ } else {
+ u32 tmp = readl(comp->regs + offset);
+
+ tmp = (tmp & ~mask) | (value & mask);
+ writel(tmp, comp->regs + offset);
+ }
+}
+
void mtk_dither_set(struct mtk_ddp_comp *comp, unsigned int bpc,
- unsigned int CFG)
+ unsigned int CFG, struct cmdq_pkt *cmdq_pkt)
{
/* If bpc equal to 0, the dithering function didn't be enabled */
if (bpc == 0)
return;
if (bpc >= MTK_MIN_BPC) {
- writel(0, comp->regs + DISP_DITHER_5);
- writel(0, comp->regs + DISP_DITHER_7);
- writel(DITHER_LSB_ERR_SHIFT_R(MTK_MAX_BPC - bpc) |
- DITHER_ADD_LSHIFT_R(MTK_MAX_BPC - bpc) |
- DITHER_NEW_BIT_MODE,
- comp->regs + DISP_DITHER_15);
- writel(DITHER_LSB_ERR_SHIFT_B(MTK_MAX_BPC - bpc) |
- DITHER_ADD_LSHIFT_B(MTK_MAX_BPC - bpc) |
- DITHER_LSB_ERR_SHIFT_G(MTK_MAX_BPC - bpc) |
- DITHER_ADD_LSHIFT_G(MTK_MAX_BPC - bpc),
- comp->regs + DISP_DITHER_16);
- writel(DISP_DITHERING, comp->regs + CFG);
+ mtk_ddp_write(cmdq_pkt, 0, comp, DISP_DITHER_5);
+ mtk_ddp_write(cmdq_pkt, 0, comp, DISP_DITHER_7);
+ mtk_ddp_write(cmdq_pkt,
+ DITHER_LSB_ERR_SHIFT_R(MTK_MAX_BPC - bpc) |
+ DITHER_ADD_LSHIFT_R(MTK_MAX_BPC - bpc) |
+ DITHER_NEW_BIT_MODE,
+ comp, DISP_DITHER_15);
+ mtk_ddp_write(cmdq_pkt,
+ DITHER_LSB_ERR_SHIFT_B(MTK_MAX_BPC - bpc) |
+ DITHER_ADD_LSHIFT_B(MTK_MAX_BPC - bpc) |
+ DITHER_LSB_ERR_SHIFT_G(MTK_MAX_BPC - bpc) |
+ DITHER_ADD_LSHIFT_G(MTK_MAX_BPC - bpc),
+ comp, DISP_DITHER_16);
+ mtk_ddp_write(cmdq_pkt, DISP_DITHERING, comp, CFG);
}
}
static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w,
unsigned int h, unsigned int vrefresh,
- unsigned int bpc)
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
- writel(w << 16 | h, comp->regs + DISP_OD_SIZE);
- writel(OD_RELAYMODE, comp->regs + DISP_OD_CFG);
- mtk_dither_set(comp, bpc, DISP_OD_CFG);
+ mtk_ddp_write(cmdq_pkt, w << 16 | h, comp, DISP_OD_SIZE);
+ mtk_ddp_write(cmdq_pkt, OD_RELAYMODE, comp, DISP_OD_CFG);
+ mtk_dither_set(comp, bpc, DISP_OD_CFG, cmdq_pkt);
}
static void mtk_od_start(struct mtk_ddp_comp *comp)
@@ -120,9 +167,9 @@ static void mtk_ufoe_start(struct mtk_ddp_comp *comp)
static void mtk_aal_config(struct mtk_ddp_comp *comp, unsigned int w,
unsigned int h, unsigned int vrefresh,
- unsigned int bpc)
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
- writel(h << 16 | w, comp->regs + DISP_AAL_SIZE);
+ mtk_ddp_write(cmdq_pkt, h << 16 | w, comp, DISP_AAL_SIZE);
}
static void mtk_aal_start(struct mtk_ddp_comp *comp)
@@ -137,10 +184,10 @@ static void mtk_aal_stop(struct mtk_ddp_comp *comp)
static void mtk_ccorr_config(struct mtk_ddp_comp *comp, unsigned int w,
unsigned int h, unsigned int vrefresh,
- unsigned int bpc)
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
- writel(h << 16 | w, comp->regs + DISP_CCORR_SIZE);
- writel(CCORR_RELAY_MODE, comp->regs + DISP_CCORR_CFG);
+ mtk_ddp_write(cmdq_pkt, h << 16 | w, comp, DISP_CCORR_SIZE);
+ mtk_ddp_write(cmdq_pkt, CCORR_RELAY_MODE, comp, DISP_CCORR_CFG);
}
static void mtk_ccorr_start(struct mtk_ddp_comp *comp)
@@ -155,10 +202,10 @@ static void mtk_ccorr_stop(struct mtk_ddp_comp *comp)
static void mtk_dither_config(struct mtk_ddp_comp *comp, unsigned int w,
unsigned int h, unsigned int vrefresh,
- unsigned int bpc)
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
- writel(h << 16 | w, comp->regs + DISP_DITHER_SIZE);
- writel(DITHER_RELAY_MODE, comp->regs + DISP_DITHER_CFG);
+ mtk_ddp_write(cmdq_pkt, h << 16 | w, comp, DISP_DITHER_SIZE);
+ mtk_ddp_write(cmdq_pkt, DITHER_RELAY_MODE, comp, DISP_DITHER_CFG);
}
static void mtk_dither_start(struct mtk_ddp_comp *comp)
@@ -173,10 +220,10 @@ static void mtk_dither_stop(struct mtk_ddp_comp *comp)
static void mtk_gamma_config(struct mtk_ddp_comp *comp, unsigned int w,
unsigned int h, unsigned int vrefresh,
- unsigned int bpc)
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
- writel(h << 16 | w, comp->regs + DISP_GAMMA_SIZE);
- mtk_dither_set(comp, bpc, DISP_GAMMA_CFG);
+ mtk_ddp_write(cmdq_pkt, h << 16 | w, comp, DISP_GAMMA_SIZE);
+ mtk_dither_set(comp, bpc, DISP_GAMMA_CFG, cmdq_pkt);
}
static void mtk_gamma_start(struct mtk_ddp_comp *comp)
@@ -190,24 +237,23 @@ static void mtk_gamma_stop(struct mtk_ddp_comp *comp)
}
static void mtk_gamma_set(struct mtk_ddp_comp *comp,
- struct drm_crtc_state *state)
+ struct drm_crtc_state *state,
+ struct cmdq_pkt *cmdq_pkt)
{
- unsigned int i, reg;
+ unsigned int i;
struct drm_color_lut *lut;
- void __iomem *lut_base;
u32 word;
if (state->gamma_lut) {
- reg = readl(comp->regs + DISP_GAMMA_CFG);
- reg = reg | GAMMA_LUT_EN;
- writel(reg, comp->regs + DISP_GAMMA_CFG);
- lut_base = comp->regs + DISP_GAMMA_LUT;
+ mtk_ddp_write_mask(cmdq_pkt, GAMMA_LUT_EN, comp,
+ DISP_GAMMA_CFG, GAMMA_LUT_EN);
lut = (struct drm_color_lut *)state->gamma_lut->data;
for (i = 0; i < MTK_LUT_SIZE; i++) {
word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
((lut[i].blue >> 6) & LUT_10BIT_MASK);
- writel(word, (lut_base + i * 4));
+ mtk_ddp_write(cmdq_pkt, word, comp,
+ DISP_GAMMA_LUT + i * 4);
}
}
}
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index ec55c7488cc3..5b0a3d48dfa6 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -69,25 +69,30 @@ enum mtk_ddp_comp_id {
};
struct mtk_ddp_comp;
-
+struct cmdq_pkt;
struct mtk_ddp_comp_funcs {
void (*config)(struct mtk_ddp_comp *comp, unsigned int w,
- unsigned int h, unsigned int vrefresh, unsigned int bpc);
+ unsigned int h, unsigned int vrefresh,
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
void (*start)(struct mtk_ddp_comp *comp);
void (*stop)(struct mtk_ddp_comp *comp);
void (*enable_vblank)(struct mtk_ddp_comp *comp, struct drm_crtc *crtc);
void (*disable_vblank)(struct mtk_ddp_comp *comp);
unsigned int (*supported_rotations)(struct mtk_ddp_comp *comp);
unsigned int (*layer_nr)(struct mtk_ddp_comp *comp);
- void (*layer_on)(struct mtk_ddp_comp *comp, unsigned int idx);
- void (*layer_off)(struct mtk_ddp_comp *comp, unsigned int idx);
+ void (*layer_on)(struct mtk_ddp_comp *comp, unsigned int idx,
+ struct cmdq_pkt *cmdq_pkt);
+ void (*layer_off)(struct mtk_ddp_comp *comp, unsigned int idx,
+ struct cmdq_pkt *cmdq_pkt);
int (*layer_check)(struct mtk_ddp_comp *comp,
unsigned int idx,
struct mtk_plane_state *state);
void (*layer_config)(struct mtk_ddp_comp *comp, unsigned int idx,
- struct mtk_plane_state *state);
+ struct mtk_plane_state *state,
+ struct cmdq_pkt *cmdq_pkt);
void (*gamma_set)(struct mtk_ddp_comp *comp,
- struct drm_crtc_state *state);
+ struct drm_crtc_state *state,
+ struct cmdq_pkt *cmdq_pkt);
void (*bgclr_in_on)(struct mtk_ddp_comp *comp);
void (*bgclr_in_off)(struct mtk_ddp_comp *comp);
};
@@ -99,14 +104,17 @@ struct mtk_ddp_comp {
struct device *dev;
enum mtk_ddp_comp_id id;
const struct mtk_ddp_comp_funcs *funcs;
+ resource_size_t regs_pa;
+ u8 subsys;
};
static inline void mtk_ddp_comp_config(struct mtk_ddp_comp *comp,
unsigned int w, unsigned int h,
- unsigned int vrefresh, unsigned int bpc)
+ unsigned int vrefresh, unsigned int bpc,
+ struct cmdq_pkt *cmdq_pkt)
{
if (comp->funcs && comp->funcs->config)
- comp->funcs->config(comp, w, h, vrefresh, bpc);
+ comp->funcs->config(comp, w, h, vrefresh, bpc, cmdq_pkt);
}
static inline void mtk_ddp_comp_start(struct mtk_ddp_comp *comp)
@@ -152,17 +160,19 @@ static inline unsigned int mtk_ddp_comp_layer_nr(struct mtk_ddp_comp *comp)
}
static inline void mtk_ddp_comp_layer_on(struct mtk_ddp_comp *comp,
- unsigned int idx)
+ unsigned int idx,
+ struct cmdq_pkt *cmdq_pkt)
{
if (comp->funcs && comp->funcs->layer_on)
- comp->funcs->layer_on(comp, idx);
+ comp->funcs->layer_on(comp, idx, cmdq_pkt);
}
static inline void mtk_ddp_comp_layer_off(struct mtk_ddp_comp *comp,
- unsigned int idx)
+ unsigned int idx,
+ struct cmdq_pkt *cmdq_pkt)
{
if (comp->funcs && comp->funcs->layer_off)
- comp->funcs->layer_off(comp, idx);
+ comp->funcs->layer_off(comp, idx, cmdq_pkt);
}
static inline int mtk_ddp_comp_layer_check(struct mtk_ddp_comp *comp,
@@ -176,17 +186,19 @@ static inline int mtk_ddp_comp_layer_check(struct mtk_ddp_comp *comp,
static inline void mtk_ddp_comp_layer_config(struct mtk_ddp_comp *comp,
unsigned int idx,
- struct mtk_plane_state *state)
+ struct mtk_plane_state *state,
+ struct cmdq_pkt *cmdq_pkt)
{
if (comp->funcs && comp->funcs->layer_config)
- comp->funcs->layer_config(comp, idx, state);
+ comp->funcs->layer_config(comp, idx, state, cmdq_pkt);
}
static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
- struct drm_crtc_state *state)
+ struct drm_crtc_state *state,
+ struct cmdq_pkt *cmdq_pkt)
{
if (comp->funcs && comp->funcs->gamma_set)
- comp->funcs->gamma_set(comp, state);
+ comp->funcs->gamma_set(comp, state, cmdq_pkt);
}
static inline void mtk_ddp_comp_bgclr_in_on(struct mtk_ddp_comp *comp)
@@ -209,6 +221,13 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *comp_node,
int mtk_ddp_comp_register(struct drm_device *drm, struct mtk_ddp_comp *comp);
void mtk_ddp_comp_unregister(struct drm_device *drm, struct mtk_ddp_comp *comp);
void mtk_dither_set(struct mtk_ddp_comp *comp, unsigned int bpc,
- unsigned int CFG);
-
+ unsigned int CFG, struct cmdq_pkt *cmdq_pkt);
+enum mtk_ddp_comp_type mtk_ddp_comp_get_type(enum mtk_ddp_comp_id comp_id);
+void mtk_ddp_write(struct cmdq_pkt *cmdq_pkt, unsigned int value,
+ struct mtk_ddp_comp *comp, unsigned int offset);
+void mtk_ddp_write_relaxed(struct cmdq_pkt *cmdq_pkt, unsigned int value,
+ struct mtk_ddp_comp *comp, unsigned int offset);
+void mtk_ddp_write_mask(struct cmdq_pkt *cmdq_pkt, unsigned int value,
+ struct mtk_ddp_comp *comp, unsigned int offset,
+ unsigned int mask);
#endif /* MTK_DRM_DDP_COMP_H */
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 7/7] drm/mediatek: apply CMDQ control flow
2019-11-26 6:29 [PATCH 0/7] drm/mediatek: fix cursor issue and apply CMDQ in Bibby Hsieh
` (5 preceding siblings ...)
2019-11-26 6:29 ` [PATCH 6/7] drm/mediatek: support CMDQ interface in ddp component Bibby Hsieh
@ 2019-11-26 6:29 ` Bibby Hsieh
6 siblings, 0 replies; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-26 6:29 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel, linux-mediatek
Cc: drinkcat, Bibby Hsieh, srv_heupstream, Yongqiang Niu,
linux-kernel, tfiga, CK Hu, Thierry Reding, Philipp Zabel,
linux-arm-kernel
Unlike other SoCs, MT8183 does not have "shadow"
registers for performaing an atomic video mode
set or page flip at vblank/vsync.
The CMDQ (Commend Queue) in MT8183 is used to help
update all relevant display controller registers
with critical time limation.
Signed-off-by: YT Shen <yt.shen@mediatek.com>
Signed-off-by: CK Hu <ck.hu@mediatek.com>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 165 ++++++++++++++++++--
drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 3 +-
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 31 ++++
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 4 +
4 files changed, 187 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 3935fac624f6..4fb346cdda79 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -12,6 +12,8 @@
#include <drm/drm_plane_helper.h>
#include <drm/drm_probe_helper.h>
#include <drm/drm_vblank.h>
+#include <linux/of_address.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
#include "mtk_drm_drv.h"
#include "mtk_drm_crtc.h"
@@ -20,6 +22,7 @@
#include "mtk_drm_gem.h"
#include "mtk_drm_plane.h"
+typedef void (*drm_cmdq_cb)(struct cmdq_cb_data data);
/**
* struct mtk_drm_crtc - MediaTek specific crtc structure.
* @base: crtc object.
@@ -42,6 +45,10 @@ struct mtk_drm_crtc {
unsigned int layer_nr;
bool pending_planes;
bool cursor_update;
+
+ struct cmdq_client *cmdq_client;
+ u32 cmdq_event;
+
void __iomem *config_regs;
const struct mtk_mmsys_reg_data *mmsys_reg_data;
struct mtk_disp_mutex *mutex;
@@ -57,6 +64,12 @@ struct mtk_crtc_state {
unsigned int pending_width;
unsigned int pending_height;
unsigned int pending_vrefresh;
+ struct cmdq_pkt *cmdq_handle;
+};
+
+struct mtk_cmdq_cb_data {
+ struct drm_crtc_state *state;
+ struct cmdq_pkt *cmdq_handle;
};
static inline struct mtk_drm_crtc *to_mtk_crtc(struct drm_crtc *c)
@@ -228,8 +241,71 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc,
WARN(1, "Failed to find component for plane %d\n", plane->index);
return NULL;
+};
+
+#ifdef CONFIG_MTK_CMDQ
+static void ddp_cmdq_cursor_cb(struct cmdq_cb_data data)
+{
+ struct mtk_cmdq_cb_data *cb_data = data.data;
+
+ cmdq_pkt_destroy(cb_data->cmdq_handle);
+ kfree(cb_data);
}
+static void ddp_cmdq_cb(struct cmdq_cb_data data)
+{
+ struct mtk_cmdq_cb_data *cb_data = data.data;
+ struct drm_crtc_state *crtc_state = cb_data->state;
+ struct drm_atomic_state *atomic_state = crtc_state->state;
+ struct drm_crtc *crtc = crtc_state->crtc;
+ struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+
+ if (mtk_crtc->pending_needs_vblank) {
+ /* cmdq_vblank_event must be read after cmdq_needs_event */
+ smp_rmb();
+
+ mtk_drm_crtc_finish_page_flip(mtk_crtc);
+ mtk_crtc->pending_needs_vblank = false;
+ }
+ mtk_atomic_state_put_queue(atomic_state);
+ cmdq_pkt_destroy(cb_data->cmdq_handle);
+ kfree(cb_data);
+}
+
+static void mtk_cmdq_acquire(struct drm_crtc *crtc)
+{
+ struct mtk_crtc_state *mtk_crtc_state =
+ to_mtk_crtc_state(crtc->state);
+ struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+
+ mtk_crtc_state->cmdq_handle =
+ cmdq_pkt_create(mtk_crtc->cmdq_client,
+ PAGE_SIZE);
+ cmdq_pkt_clear_event(mtk_crtc_state->cmdq_handle,
+ mtk_crtc->cmdq_event);
+ cmdq_pkt_wfe(mtk_crtc_state->cmdq_handle, mtk_crtc->cmdq_event);
+}
+
+static void mtk_cmdq_release(struct drm_crtc *crtc,
+ struct drm_crtc_state *old_crtc_state,
+ drm_cmdq_cb cb)
+{
+ struct mtk_crtc_state *mtk_crtc_state =
+ to_mtk_crtc_state(crtc->state);
+ struct mtk_cmdq_cb_data *cb_data;
+
+ cb_data = kmalloc(sizeof(*cb_data), GFP_KERNEL);
+ if (!cb_data) {
+ DRM_DEV_ERROR(crtc->dev->dev, "Failed to alloc cb_data\n");
+ return;
+ }
+
+ cb_data->cmdq_handle = mtk_crtc_state->cmdq_handle;
+ cb_data->state = old_crtc_state;
+ cmdq_pkt_flush_async(mtk_crtc_state->cmdq_handle,
+ cb, cb_data);
+}
+#endif
static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
{
struct drm_crtc *crtc = &mtk_crtc->base;
@@ -371,7 +447,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
{
struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
- struct drm_atomic_state *atomic_state = mtk_crtc->old_crtc_state->state;
+ struct drm_atomic_state *atomic_state;
struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
unsigned int i;
@@ -385,7 +461,8 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
if (state->pending_config) {
mtk_ddp_comp_config(comp, state->pending_width,
state->pending_height,
- state->pending_vrefresh, 0, NULL);
+ state->pending_vrefresh, 0,
+ state->cmdq_handle);
state->pending_config = false;
}
@@ -405,12 +482,15 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
if (comp)
mtk_ddp_comp_layer_config(comp, local_layer,
- plane_state, NULL);
+ plane_state,
+ state->cmdq_handle);
plane_state->pending.config = false;
}
mtk_crtc->pending_planes = false;
- if (!mtk_crtc->cursor_update)
+ if (!mtk_crtc->cursor_update) {
+ atomic_state = mtk_crtc->old_crtc_state->state;
mtk_atomic_state_put_queue(atomic_state);
+ }
mtk_crtc->cursor_update = false;
}
}
@@ -454,14 +534,40 @@ void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
}
}
mtk_crtc->pending_planes = true;
+ mtk_crtc->cursor_update = true;
if (priv->data->shadow_register) {
mtk_disp_mutex_acquire(mtk_crtc->mutex);
mtk_crtc_ddp_config(crtc);
mtk_disp_mutex_release(mtk_crtc->mutex);
}
+#ifdef CONFIG_MTK_CMDQ
+ if (mtk_crtc->cmdq_client) {
+ mtk_cmdq_acquire(crtc);
+ mtk_crtc_ddp_config(crtc);
+ mtk_cmdq_release(crtc, NULL, ddp_cmdq_cursor_cb);
+ }
+#endif
mutex_unlock(&priv->hw_lock);
}
+void mtk_drm_crtc_plane_update(struct drm_crtc *crtc, struct drm_plane *plane,
+ struct mtk_plane_state *state)
+{
+ struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+ struct mtk_ddp_comp *comp;
+ struct drm_crtc_state *crtc_state = crtc->state;
+ struct mtk_crtc_state *mtk_crtc_state = to_mtk_crtc_state(crtc_state);
+ struct cmdq_pkt *cmdq_handle = mtk_crtc_state->cmdq_handle;
+ unsigned int local_layer;
+
+ if (mtk_crtc->cmdq_client) {
+ comp = mtk_drm_ddp_comp_for_plane(crtc, plane,
+ &local_layer);
+ mtk_ddp_comp_layer_config(comp, local_layer, state,
+ cmdq_handle);
+ }
+}
+
static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_crtc_state *old_state)
{
@@ -538,21 +644,39 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
WARN_ON(mtk_crtc->event);
mtk_crtc->event = state->base.event;
state->base.event = NULL;
+ /* Make sure the above parameter is set before update */
+ smp_wmb();
+ mtk_crtc->pending_needs_vblank = true;
}
spin_unlock_irq(&crtc->dev->event_lock);
+#ifdef CONFIG_MTK_CMDQ
+ if (mtk_crtc->cmdq_client)
+ mtk_cmdq_acquire(crtc);
+#endif
}
static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
{
struct drm_atomic_state *old_atomic_state = old_crtc_state->state;
+ struct mtk_crtc_state *mtk_crtc_state = to_mtk_crtc_state(crtc->state);
struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
struct mtk_drm_private *priv = crtc->dev->dev_private;
unsigned int pending_planes = 0;
int i;
- if (mtk_crtc->event)
- mtk_crtc->pending_needs_vblank = true;
+ if (crtc->state->color_mgmt_changed)
+ for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
+ mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i],
+ crtc->state,
+ mtk_crtc_state->cmdq_handle);
+#ifdef CONFIG_MTK_CMDQ
+ if (mtk_crtc->cmdq_client) {
+ drm_atomic_state_get(old_atomic_state);
+ mtk_cmdq_release(crtc, old_crtc_state, ddp_cmdq_cb);
+ return;
+ }
+#endif
for (i = 0; i < mtk_crtc->layer_nr; i++) {
struct drm_plane *plane = &mtk_crtc->planes[i];
struct mtk_plane_state *plane_state;
@@ -569,10 +693,6 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
drm_atomic_state_get(old_atomic_state);
mtk_crtc->old_crtc_state = old_crtc_state;
}
- if (crtc->state->color_mgmt_changed)
- for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
- mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i],
- crtc->state, NULL);
if (priv->data->shadow_register) {
mtk_disp_mutex_acquire(mtk_crtc->mutex);
@@ -628,10 +748,13 @@ void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *comp)
struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
struct mtk_drm_private *priv = crtc->dev->dev_private;
- if (!priv->data->shadow_register)
- mtk_crtc_ddp_config(crtc);
-
- mtk_drm_finish_page_flip(mtk_crtc);
+ if (mtk_crtc->cmdq_client) {
+ drm_crtc_handle_vblank(crtc);
+ } else {
+ if (!priv->data->shadow_register)
+ mtk_crtc_ddp_config(crtc);
+ mtk_drm_finish_page_flip(mtk_crtc);
+ }
}
static int mtk_drm_crtc_num_comp_planes(struct mtk_drm_crtc *mtk_crtc,
@@ -770,6 +893,18 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
drm_mode_crtc_set_gamma_size(&mtk_crtc->base, MTK_LUT_SIZE);
drm_crtc_enable_color_mgmt(&mtk_crtc->base, 0, false, MTK_LUT_SIZE);
priv->num_pipes++;
-
+#ifdef CONFIG_MTK_CMDQ
+ mtk_crtc->cmdq_client =
+ cmdq_mbox_create(dev, drm_crtc_index(&mtk_crtc->base),
+ 2000);
+ of_property_read_u32_index(dev->of_node, "mediatek,gce-events",
+ drm_crtc_index(&mtk_crtc->base),
+ &mtk_crtc->cmdq_event);
+ if (IS_ERR(mtk_crtc->cmdq_client)) {
+ dev_dbg(dev, "mtk_crtc %d failed to create mailbox client, writing register by CPU now\n",
+ drm_crtc_index(&mtk_crtc->base));
+ mtk_crtc->cmdq_client = NULL;
+ }
+#endif
return 0;
}
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
index 42a3f650f564..64e6cf969626 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
@@ -21,7 +21,8 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
unsigned int path_len);
int mtk_drm_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane *plane,
struct mtk_plane_state *state);
-
+void mtk_drm_crtc_plane_update(struct drm_crtc *crtc, struct drm_plane *plane,
+ struct mtk_plane_state *state);
void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
struct drm_plane_state *plane_state);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 6d0f349ddf82..9cc12af2bc06 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -370,6 +370,9 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
const struct mtk_ddp_comp_funcs *funcs)
{
struct platform_device *comp_pdev;
+ struct resource res;
+ struct cmdq_client_reg *cmdq_reg;
+ int ret = 0;
if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX)
return -EINVAL;
@@ -404,6 +407,34 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
}
comp->dev = &comp_pdev->dev;
+#ifdef CONFIG_MTK_CMDQ
+ if (of_address_to_resource(node, 0, &res) != 0) {
+ dev_err(dev, "Missing reg in %s node\n",
+ node->full_name);
+ return -EINVAL;
+ }
+ comp->regs_pa = res.start;
+
+ comp_pdev = of_find_device_by_node(node);
+ if (!comp_pdev) {
+ dev_warn(dev, "Waiting for component device %s\n",
+ node->full_name);
+ return -EPROBE_DEFER;
+ }
+
+ cmdq_reg = kzalloc(sizeof(*cmdq_reg), GFP_KERNEL);
+ if (!cmdq_reg)
+ return -EINVAL;
+
+ ret = cmdq_dev_get_client_reg(&comp_pdev->dev, cmdq_reg, 0);
+ if (ret != 0)
+ dev_dbg(&comp_pdev->dev,
+ "get mediatek,gce-client-reg fail!\n");
+ else
+ comp->subsys = cmdq_reg->subsys;
+
+ kfree(cmdq_reg);
+#endif
return 0;
}
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index d7a8853d94a1..8e857ef512a2 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -189,6 +189,8 @@ static void mtk_plane_atomic_update(struct drm_plane *plane,
if (state->pending.cursor_update)
state->pending.cursor_dirty = true;
+ else
+ mtk_drm_crtc_plane_update(crtc, plane, state);
}
static void mtk_plane_atomic_disable(struct drm_plane *plane,
@@ -199,6 +201,8 @@ static void mtk_plane_atomic_disable(struct drm_plane *plane,
state->pending.enable = false;
wmb(); /* Make sure the above parameter is set before update */
state->pending.dirty = true;
+ /* Fetch CRTC from old plane state when disabling. */
+ mtk_drm_crtc_plane_update(old_state->crtc, plane, state);
}
static const struct drm_plane_helper_funcs mtk_plane_helper_funcs = {
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/7] drm/mediatek: fix atomic_state reference counting
2019-11-26 6:29 ` [PATCH 1/7] drm/mediatek: fix atomic_state reference counting Bibby Hsieh
@ 2019-11-26 8:49 ` Daniel Vetter
2019-11-27 1:41 ` Bibby Hsieh
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2019-11-26 8:49 UTC (permalink / raw)
To: Bibby Hsieh
Cc: drinkcat, srv_heupstream, David Airlie, Daniel Vetter,
linux-kernel, dri-devel, tfiga, CK Hu, Thierry Reding,
linux-mediatek, Philipp Zabel, Matthias Brugger,
linux-arm-kernel
On Tue, Nov 26, 2019 at 02:29:26PM +0800, Bibby Hsieh wrote:
> The DRM core takes care of all atomic state refcounting.
> However, mediatek drm defers some work that accesses planes
> and plane_states in drm_atomic_state, and must therefore
> keep its own atomic state references until this work complete.
>
> We take the atomic_state reference in atomic_fulsh() and ensure all the
> information in atomic_state already was updated in hardware for
> showing on screen and then schedules unreference_work to drop references
> on atomic_state.
>
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
This looks strange. For one you implement your own reference counting - if
drivers have a need for drm_atomic_state_put_irq then I
think we should implement this in the core code.
The other bit is that atomic commits are meant to simply wait for
everything to finish - commit_tail doesn't hold locks, it's only ordered
through drm_crtc_commit events (at least with the async implementation in
the helpers), so you can just block there until your interrupt handler is
done processing the commit. Depending how you want to do this you might
want to wait before or after drm_atomic_helper_commit_hw_done().
-Daniel
> ---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 11 +++-
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 79 +++++++++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 9 +++
> 3 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 29d0582e90e9..68b92adc96bb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -7,7 +7,7 @@
> #include <linux/pm_runtime.h>
>
> #include <asm/barrier.h>
> -
> +#include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_plane_helper.h>
> #include <drm/drm_probe_helper.h>
> @@ -47,6 +47,7 @@ struct mtk_drm_crtc {
> struct mtk_disp_mutex *mutex;
> unsigned int ddp_comp_nr;
> struct mtk_ddp_comp **ddp_comp;
> + struct drm_crtc_state *old_crtc_state;
> };
>
> struct mtk_crtc_state {
> @@ -362,6 +363,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
> static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> {
> struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> + struct drm_atomic_state *atomic_state = mtk_crtc->old_crtc_state->state;
> struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
> struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> unsigned int i;
> @@ -399,6 +401,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> plane_state->pending.config = false;
> }
> mtk_crtc->pending_planes = false;
> + mtk_atomic_state_put_queue(atomic_state);
> }
> }
>
> @@ -494,6 +497,7 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
> static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> struct drm_crtc_state *old_crtc_state)
> {
> + struct drm_atomic_state *old_atomic_state = old_crtc_state->state;
> struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> struct mtk_drm_private *priv = crtc->dev->dev_private;
> unsigned int pending_planes = 0;
> @@ -512,8 +516,11 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> pending_planes |= BIT(i);
> }
> }
> - if (pending_planes)
> + if (pending_planes) {
> mtk_crtc->pending_planes = true;
> + drm_atomic_state_get(old_atomic_state);
> + mtk_crtc->old_crtc_state = old_crtc_state;
> + }
> if (crtc->state->color_mgmt_changed)
> for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
> mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 6588dc6dd5e3..6c68283b6124 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -115,10 +115,85 @@ static int mtk_atomic_commit(struct drm_device *drm,
> return 0;
> }
>
> +struct mtk_atomic_state {
> + struct drm_atomic_state base;
> + struct list_head list;
> +};
> +
> +static inline struct mtk_atomic_state *to_mtk_state(struct drm_atomic_state *s)
> +{
> + return container_of(s, struct mtk_atomic_state, base);
> +}
> +
> +void mtk_atomic_state_put_queue(struct drm_atomic_state *state)
> +{
> + struct drm_device *drm = state->dev;
> + struct mtk_drm_private *mtk_drm = drm->dev_private;
> + struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> + list_add_tail(&mtk_state->list, &mtk_drm->unreference.list);
> + spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> +
> + schedule_work(&mtk_drm->unreference.work);
> +}
> +
> +static void mtk_unreference_work(struct work_struct *work)
> +{
> + struct mtk_drm_private *mtk_drm = container_of(work,
> + struct mtk_drm_private, unreference.work);
> + unsigned long flags;
> + struct mtk_atomic_state *state, *tmp;
> +
> + /*
> + * framebuffers cannot be unreferenced in atomic context.
> + * Therefore, only hold the spinlock when iterating unreference_list,
> + * and drop it when doing the unreference.
> + */
> + spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> + list_for_each_entry_safe(state, tmp, &mtk_drm->unreference.list, list) {
> + list_del(&state->list);
> + spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> + drm_atomic_state_put(&state->base);
> + spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> + }
> + spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> +}
> +
> +static struct drm_atomic_state *
> + mtk_drm_atomic_state_alloc(struct drm_device *dev)
> +{
> + struct mtk_atomic_state *mtk_state;
> +
> + mtk_state = kzalloc(sizeof(*mtk_state), GFP_KERNEL);
> + if (!mtk_state)
> + return NULL;
> +
> + if (drm_atomic_state_init(dev, &mtk_state->base) < 0) {
> + kfree(mtk_state);
> + return NULL;
> + }
> +
> + INIT_LIST_HEAD(&mtk_state->list);
> +
> + return &mtk_state->base;
> +}
> +
> +static void mtk_drm_atomic_state_free(struct drm_atomic_state *state)
> +{
> + struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> +
> + drm_atomic_state_default_release(state);
> + kfree(mtk_state);
> +}
> +
> static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
> .fb_create = mtk_drm_mode_fb_create,
> .atomic_check = drm_atomic_helper_check,
> .atomic_commit = mtk_atomic_commit,
> + .atomic_state_alloc = mtk_drm_atomic_state_alloc,
> + .atomic_state_free = mtk_drm_atomic_state_free
> };
>
> static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
> @@ -337,6 +412,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> drm_kms_helper_poll_init(drm);
> drm_mode_config_reset(drm);
>
> + INIT_WORK(&private->unreference.work, mtk_unreference_work);
> + INIT_LIST_HEAD(&private->unreference.list);
> + spin_lock_init(&private->unreference.lock);
> +
> return 0;
>
> err_unset_dma_parms:
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index b6a82728d563..c37d835cf949 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -55,6 +55,13 @@ struct mtk_drm_private {
>
> struct drm_atomic_state *suspend_state;
>
> + struct {
> + struct work_struct work;
> + struct list_head list;
> + /* lock for unreference list */
> + spinlock_t lock;
> + } unreference;
> +
> bool dma_parms_allocated;
> };
>
> @@ -66,4 +73,6 @@ extern struct platform_driver mtk_dpi_driver;
> extern struct platform_driver mtk_dsi_driver;
> extern struct platform_driver mtk_mipi_tx_driver;
>
> +void mtk_atomic_state_put_queue(struct drm_atomic_state *state);
> +
> #endif /* MTK_DRM_DRV_H */
> --
> 2.18.0
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/7] drm/mediatek: fix atomic_state reference counting
2019-11-26 8:49 ` Daniel Vetter
@ 2019-11-27 1:41 ` Bibby Hsieh
2019-11-27 9:56 ` Daniel Vetter
0 siblings, 1 reply; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-27 1:41 UTC (permalink / raw)
To: Daniel Vetter
Cc: drinkcat, srv_heupstream, David Airlie, Daniel Vetter,
linux-kernel, dri-devel, tfiga, CK Hu, Thierry Reding,
linux-mediatek, Philipp Zabel, Matthias Brugger,
linux-arm-kernel
On Tue, 2019-11-26 at 09:49 +0100, Daniel Vetter wrote:
> On Tue, Nov 26, 2019 at 02:29:26PM +0800, Bibby Hsieh wrote:
> > The DRM core takes care of all atomic state refcounting.
> > However, mediatek drm defers some work that accesses planes
> > and plane_states in drm_atomic_state, and must therefore
> > keep its own atomic state references until this work complete.
> >
> > We take the atomic_state reference in atomic_fulsh() and ensure all the
> > information in atomic_state already was updated in hardware for
> > showing on screen and then schedules unreference_work to drop references
> > on atomic_state.
> >
> > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> >
> > Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
>
> This looks strange. For one you implement your own reference counting - if
> drivers have a need for drm_atomic_state_put_irq then I
> think we should implement this in the core code.
>
> The other bit is that atomic commits are meant to simply wait for
> everything to finish - commit_tail doesn't hold locks, it's only ordered
> through drm_crtc_commit events (at least with the async implementation in
> the helpers), so you can just block there until your interrupt handler is
> done processing the commit. Depending how you want to do this you might
> want to wait before or after drm_atomic_helper_commit_hw_done().
OK, I will try to add a simple wait/completion before
drm_atomic_helper_commit_hw_done() until the commit was processed.
Thanks.
Bibby
> -Daniel
>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 11 +++-
> > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 79 +++++++++++++++++++++++++
> > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 9 +++
> > 3 files changed, 97 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 29d0582e90e9..68b92adc96bb 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -7,7 +7,7 @@
> > #include <linux/pm_runtime.h>
> >
> > #include <asm/barrier.h>
> > -
> > +#include <drm/drm_atomic.h>
> > #include <drm/drm_atomic_helper.h>
> > #include <drm/drm_plane_helper.h>
> > #include <drm/drm_probe_helper.h>
> > @@ -47,6 +47,7 @@ struct mtk_drm_crtc {
> > struct mtk_disp_mutex *mutex;
> > unsigned int ddp_comp_nr;
> > struct mtk_ddp_comp **ddp_comp;
> > + struct drm_crtc_state *old_crtc_state;
> > };
> >
> > struct mtk_crtc_state {
> > @@ -362,6 +363,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
> > static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> > {
> > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > + struct drm_atomic_state *atomic_state = mtk_crtc->old_crtc_state->state;
> > struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
> > struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> > unsigned int i;
> > @@ -399,6 +401,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> > plane_state->pending.config = false;
> > }
> > mtk_crtc->pending_planes = false;
> > + mtk_atomic_state_put_queue(atomic_state);
> > }
> > }
> >
> > @@ -494,6 +497,7 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
> > static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> > struct drm_crtc_state *old_crtc_state)
> > {
> > + struct drm_atomic_state *old_atomic_state = old_crtc_state->state;
> > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > struct mtk_drm_private *priv = crtc->dev->dev_private;
> > unsigned int pending_planes = 0;
> > @@ -512,8 +516,11 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> > pending_planes |= BIT(i);
> > }
> > }
> > - if (pending_planes)
> > + if (pending_planes) {
> > mtk_crtc->pending_planes = true;
> > + drm_atomic_state_get(old_atomic_state);
> > + mtk_crtc->old_crtc_state = old_crtc_state;
> > + }
> > if (crtc->state->color_mgmt_changed)
> > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
> > mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > index 6588dc6dd5e3..6c68283b6124 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > @@ -115,10 +115,85 @@ static int mtk_atomic_commit(struct drm_device *drm,
> > return 0;
> > }
> >
> > +struct mtk_atomic_state {
> > + struct drm_atomic_state base;
> > + struct list_head list;
> > +};
> > +
> > +static inline struct mtk_atomic_state *to_mtk_state(struct drm_atomic_state *s)
> > +{
> > + return container_of(s, struct mtk_atomic_state, base);
> > +}
> > +
> > +void mtk_atomic_state_put_queue(struct drm_atomic_state *state)
> > +{
> > + struct drm_device *drm = state->dev;
> > + struct mtk_drm_private *mtk_drm = drm->dev_private;
> > + struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> > + list_add_tail(&mtk_state->list, &mtk_drm->unreference.list);
> > + spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> > +
> > + schedule_work(&mtk_drm->unreference.work);
> > +}
> > +
> > +static void mtk_unreference_work(struct work_struct *work)
> > +{
> > + struct mtk_drm_private *mtk_drm = container_of(work,
> > + struct mtk_drm_private, unreference.work);
> > + unsigned long flags;
> > + struct mtk_atomic_state *state, *tmp;
> > +
> > + /*
> > + * framebuffers cannot be unreferenced in atomic context.
> > + * Therefore, only hold the spinlock when iterating unreference_list,
> > + * and drop it when doing the unreference.
> > + */
> > + spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> > + list_for_each_entry_safe(state, tmp, &mtk_drm->unreference.list, list) {
> > + list_del(&state->list);
> > + spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> > + drm_atomic_state_put(&state->base);
> > + spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> > + }
> > + spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> > +}
> > +
> > +static struct drm_atomic_state *
> > + mtk_drm_atomic_state_alloc(struct drm_device *dev)
> > +{
> > + struct mtk_atomic_state *mtk_state;
> > +
> > + mtk_state = kzalloc(sizeof(*mtk_state), GFP_KERNEL);
> > + if (!mtk_state)
> > + return NULL;
> > +
> > + if (drm_atomic_state_init(dev, &mtk_state->base) < 0) {
> > + kfree(mtk_state);
> > + return NULL;
> > + }
> > +
> > + INIT_LIST_HEAD(&mtk_state->list);
> > +
> > + return &mtk_state->base;
> > +}
> > +
> > +static void mtk_drm_atomic_state_free(struct drm_atomic_state *state)
> > +{
> > + struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> > +
> > + drm_atomic_state_default_release(state);
> > + kfree(mtk_state);
> > +}
> > +
> > static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
> > .fb_create = mtk_drm_mode_fb_create,
> > .atomic_check = drm_atomic_helper_check,
> > .atomic_commit = mtk_atomic_commit,
> > + .atomic_state_alloc = mtk_drm_atomic_state_alloc,
> > + .atomic_state_free = mtk_drm_atomic_state_free
> > };
> >
> > static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
> > @@ -337,6 +412,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> > drm_kms_helper_poll_init(drm);
> > drm_mode_config_reset(drm);
> >
> > + INIT_WORK(&private->unreference.work, mtk_unreference_work);
> > + INIT_LIST_HEAD(&private->unreference.list);
> > + spin_lock_init(&private->unreference.lock);
> > +
> > return 0;
> >
> > err_unset_dma_parms:
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > index b6a82728d563..c37d835cf949 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > @@ -55,6 +55,13 @@ struct mtk_drm_private {
> >
> > struct drm_atomic_state *suspend_state;
> >
> > + struct {
> > + struct work_struct work;
> > + struct list_head list;
> > + /* lock for unreference list */
> > + spinlock_t lock;
> > + } unreference;
> > +
> > bool dma_parms_allocated;
> > };
> >
> > @@ -66,4 +73,6 @@ extern struct platform_driver mtk_dpi_driver;
> > extern struct platform_driver mtk_dsi_driver;
> > extern struct platform_driver mtk_mipi_tx_driver;
> >
> > +void mtk_atomic_state_put_queue(struct drm_atomic_state *state);
> > +
> > #endif /* MTK_DRM_DRV_H */
> > --
> > 2.18.0
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/7] drm/mediatek: fix atomic_state reference counting
2019-11-27 1:41 ` Bibby Hsieh
@ 2019-11-27 9:56 ` Daniel Vetter
2019-11-28 2:34 ` Bibby Hsieh
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2019-11-27 9:56 UTC (permalink / raw)
To: Bibby Hsieh
Cc: drinkcat, Philipp Zabel, srv_heupstream, David Airlie,
Daniel Vetter, linux-kernel, dri-devel, tfiga, CK Hu,
Thierry Reding, linux-mediatek, Daniel Vetter, Matthias Brugger,
linux-arm-kernel
On Wed, Nov 27, 2019 at 09:41:52AM +0800, Bibby Hsieh wrote:
> On Tue, 2019-11-26 at 09:49 +0100, Daniel Vetter wrote:
> > On Tue, Nov 26, 2019 at 02:29:26PM +0800, Bibby Hsieh wrote:
> > > The DRM core takes care of all atomic state refcounting.
> > > However, mediatek drm defers some work that accesses planes
> > > and plane_states in drm_atomic_state, and must therefore
> > > keep its own atomic state references until this work complete.
> > >
> > > We take the atomic_state reference in atomic_fulsh() and ensure all the
> > > information in atomic_state already was updated in hardware for
> > > showing on screen and then schedules unreference_work to drop references
> > > on atomic_state.
> > >
> > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> > >
> > > Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> >
> > This looks strange. For one you implement your own reference counting - if
> > drivers have a need for drm_atomic_state_put_irq then I
> > think we should implement this in the core code.
> >
> > The other bit is that atomic commits are meant to simply wait for
> > everything to finish - commit_tail doesn't hold locks, it's only ordered
> > through drm_crtc_commit events (at least with the async implementation in
> > the helpers), so you can just block there until your interrupt handler is
> > done processing the commit. Depending how you want to do this you might
> > want to wait before or after drm_atomic_helper_commit_hw_done().
>
> OK, I will try to add a simple wait/completion before
> drm_atomic_helper_commit_hw_done() until the commit was processed.
Note that waiting before hw_done stalls the next commit. You only want to
do that for e.g. irq handler that programs registers at vblank. If you
have some cleanup work that runs after the new frame is already showing,
you should wait for that _after_ hw_done, to avoid stalling the next flip.
Also if you stall after hw_done, you need to make either make sure you
handle concurrency between multiple commits for that part yourself (since
anything after hw_done isn't ordered anymore by the default helper stuff).
-Daniel
>
> Thanks.
>
> Bibby
> > -Daniel
> >
> > > ---
> > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 11 +++-
> > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 79 +++++++++++++++++++++++++
> > > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 9 +++
> > > 3 files changed, 97 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > index 29d0582e90e9..68b92adc96bb 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > @@ -7,7 +7,7 @@
> > > #include <linux/pm_runtime.h>
> > >
> > > #include <asm/barrier.h>
> > > -
> > > +#include <drm/drm_atomic.h>
> > > #include <drm/drm_atomic_helper.h>
> > > #include <drm/drm_plane_helper.h>
> > > #include <drm/drm_probe_helper.h>
> > > @@ -47,6 +47,7 @@ struct mtk_drm_crtc {
> > > struct mtk_disp_mutex *mutex;
> > > unsigned int ddp_comp_nr;
> > > struct mtk_ddp_comp **ddp_comp;
> > > + struct drm_crtc_state *old_crtc_state;
> > > };
> > >
> > > struct mtk_crtc_state {
> > > @@ -362,6 +363,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
> > > static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> > > {
> > > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > > + struct drm_atomic_state *atomic_state = mtk_crtc->old_crtc_state->state;
> > > struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
> > > struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> > > unsigned int i;
> > > @@ -399,6 +401,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> > > plane_state->pending.config = false;
> > > }
> > > mtk_crtc->pending_planes = false;
> > > + mtk_atomic_state_put_queue(atomic_state);
> > > }
> > > }
> > >
> > > @@ -494,6 +497,7 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
> > > static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> > > struct drm_crtc_state *old_crtc_state)
> > > {
> > > + struct drm_atomic_state *old_atomic_state = old_crtc_state->state;
> > > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > > struct mtk_drm_private *priv = crtc->dev->dev_private;
> > > unsigned int pending_planes = 0;
> > > @@ -512,8 +516,11 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> > > pending_planes |= BIT(i);
> > > }
> > > }
> > > - if (pending_planes)
> > > + if (pending_planes) {
> > > mtk_crtc->pending_planes = true;
> > > + drm_atomic_state_get(old_atomic_state);
> > > + mtk_crtc->old_crtc_state = old_crtc_state;
> > > + }
> > > if (crtc->state->color_mgmt_changed)
> > > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
> > > mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > index 6588dc6dd5e3..6c68283b6124 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > @@ -115,10 +115,85 @@ static int mtk_atomic_commit(struct drm_device *drm,
> > > return 0;
> > > }
> > >
> > > +struct mtk_atomic_state {
> > > + struct drm_atomic_state base;
> > > + struct list_head list;
> > > +};
> > > +
> > > +static inline struct mtk_atomic_state *to_mtk_state(struct drm_atomic_state *s)
> > > +{
> > > + return container_of(s, struct mtk_atomic_state, base);
> > > +}
> > > +
> > > +void mtk_atomic_state_put_queue(struct drm_atomic_state *state)
> > > +{
> > > + struct drm_device *drm = state->dev;
> > > + struct mtk_drm_private *mtk_drm = drm->dev_private;
> > > + struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> > > + list_add_tail(&mtk_state->list, &mtk_drm->unreference.list);
> > > + spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> > > +
> > > + schedule_work(&mtk_drm->unreference.work);
> > > +}
> > > +
> > > +static void mtk_unreference_work(struct work_struct *work)
> > > +{
> > > + struct mtk_drm_private *mtk_drm = container_of(work,
> > > + struct mtk_drm_private, unreference.work);
> > > + unsigned long flags;
> > > + struct mtk_atomic_state *state, *tmp;
> > > +
> > > + /*
> > > + * framebuffers cannot be unreferenced in atomic context.
> > > + * Therefore, only hold the spinlock when iterating unreference_list,
> > > + * and drop it when doing the unreference.
> > > + */
> > > + spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> > > + list_for_each_entry_safe(state, tmp, &mtk_drm->unreference.list, list) {
> > > + list_del(&state->list);
> > > + spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> > > + drm_atomic_state_put(&state->base);
> > > + spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> > > + }
> > > + spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> > > +}
> > > +
> > > +static struct drm_atomic_state *
> > > + mtk_drm_atomic_state_alloc(struct drm_device *dev)
> > > +{
> > > + struct mtk_atomic_state *mtk_state;
> > > +
> > > + mtk_state = kzalloc(sizeof(*mtk_state), GFP_KERNEL);
> > > + if (!mtk_state)
> > > + return NULL;
> > > +
> > > + if (drm_atomic_state_init(dev, &mtk_state->base) < 0) {
> > > + kfree(mtk_state);
> > > + return NULL;
> > > + }
> > > +
> > > + INIT_LIST_HEAD(&mtk_state->list);
> > > +
> > > + return &mtk_state->base;
> > > +}
> > > +
> > > +static void mtk_drm_atomic_state_free(struct drm_atomic_state *state)
> > > +{
> > > + struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> > > +
> > > + drm_atomic_state_default_release(state);
> > > + kfree(mtk_state);
> > > +}
> > > +
> > > static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
> > > .fb_create = mtk_drm_mode_fb_create,
> > > .atomic_check = drm_atomic_helper_check,
> > > .atomic_commit = mtk_atomic_commit,
> > > + .atomic_state_alloc = mtk_drm_atomic_state_alloc,
> > > + .atomic_state_free = mtk_drm_atomic_state_free
> > > };
> > >
> > > static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
> > > @@ -337,6 +412,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> > > drm_kms_helper_poll_init(drm);
> > > drm_mode_config_reset(drm);
> > >
> > > + INIT_WORK(&private->unreference.work, mtk_unreference_work);
> > > + INIT_LIST_HEAD(&private->unreference.list);
> > > + spin_lock_init(&private->unreference.lock);
> > > +
> > > return 0;
> > >
> > > err_unset_dma_parms:
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > index b6a82728d563..c37d835cf949 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > @@ -55,6 +55,13 @@ struct mtk_drm_private {
> > >
> > > struct drm_atomic_state *suspend_state;
> > >
> > > + struct {
> > > + struct work_struct work;
> > > + struct list_head list;
> > > + /* lock for unreference list */
> > > + spinlock_t lock;
> > > + } unreference;
> > > +
> > > bool dma_parms_allocated;
> > > };
> > >
> > > @@ -66,4 +73,6 @@ extern struct platform_driver mtk_dpi_driver;
> > > extern struct platform_driver mtk_dsi_driver;
> > > extern struct platform_driver mtk_mipi_tx_driver;
> > >
> > > +void mtk_atomic_state_put_queue(struct drm_atomic_state *state);
> > > +
> > > #endif /* MTK_DRM_DRV_H */
> > > --
> > > 2.18.0
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/7] drm/mediatek: fix atomic_state reference counting
2019-11-27 9:56 ` Daniel Vetter
@ 2019-11-28 2:34 ` Bibby Hsieh
0 siblings, 0 replies; 12+ messages in thread
From: Bibby Hsieh @ 2019-11-28 2:34 UTC (permalink / raw)
To: Daniel Vetter
Cc: drinkcat, srv_heupstream, David Airlie, Daniel Vetter,
linux-kernel, dri-devel, tfiga, CK Hu (胡俊光),
Thierry Reding, linux-mediatek, Philipp Zabel, Matthias Brugger,
linux-arm-kernel
On Wed, 2019-11-27 at 17:56 +0800, Daniel Vetter wrote:
> On Wed, Nov 27, 2019 at 09:41:52AM +0800, Bibby Hsieh wrote:
> > On Tue, 2019-11-26 at 09:49 +0100, Daniel Vetter wrote:
> > > On Tue, Nov 26, 2019 at 02:29:26PM +0800, Bibby Hsieh wrote:
> > > > The DRM core takes care of all atomic state refcounting.
> > > > However, mediatek drm defers some work that accesses planes
> > > > and plane_states in drm_atomic_state, and must therefore
> > > > keep its own atomic state references until this work complete.
> > > >
> > > > We take the atomic_state reference in atomic_fulsh() and ensure all the
> > > > information in atomic_state already was updated in hardware for
> > > > showing on screen and then schedules unreference_work to drop references
> > > > on atomic_state.
> > > >
> > > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> > > >
> > > > Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> > >
> > > This looks strange. For one you implement your own reference counting - if
> > > drivers have a need for drm_atomic_state_put_irq then I
> > > think we should implement this in the core code.
> > >
> > > The other bit is that atomic commits are meant to simply wait for
> > > everything to finish - commit_tail doesn't hold locks, it's only ordered
> > > through drm_crtc_commit events (at least with the async implementation in
> > > the helpers), so you can just block there until your interrupt handler is
> > > done processing the commit. Depending how you want to do this you might
> > > want to wait before or after drm_atomic_helper_commit_hw_done().
> >
> > OK, I will try to add a simple wait/completion before
> > drm_atomic_helper_commit_hw_done() until the commit was processed.
>
> Note that waiting before hw_done stalls the next commit. You only want to
> do that for e.g. irq handler that programs registers at vblank. If you
> have some cleanup work that runs after the new frame is already showing,
> you should wait for that _after_ hw_done, to avoid stalling the next flip.
>
> Also if you stall after hw_done, you need to make either make sure you
> handle concurrency between multiple commits for that part yourself (since
> anything after hw_done isn't ordered anymore by the default helper stuff).
> -Daniel
Thanks for the friendly reminder. :P
Actually, this patch was created 2 years ago. We wanted to fix some
issue before. But I test it and do not see the issue we met before on my
platform after I remove this patch. I'm going to send all the patches
w/o "drm/mediatek: fix atomic_state reference counting" again today.
Bibby
>
> >
> > Thanks.
> >
> > Bibby
> > > -Daniel
> > >
> > > > ---
> > > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 11 +++-
> > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 79 +++++++++++++++++++++++++
> > > > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 9 +++
> > > > 3 files changed, 97 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > > index 29d0582e90e9..68b92adc96bb 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > > @@ -7,7 +7,7 @@
> > > > #include <linux/pm_runtime.h>
> > > >
> > > > #include <asm/barrier.h>
> > > > -
> > > > +#include <drm/drm_atomic.h>
> > > > #include <drm/drm_atomic_helper.h>
> > > > #include <drm/drm_plane_helper.h>
> > > > #include <drm/drm_probe_helper.h>
> > > > @@ -47,6 +47,7 @@ struct mtk_drm_crtc {
> > > > struct mtk_disp_mutex*mutex;
> > > > unsigned intddp_comp_nr;
> > > > struct mtk_ddp_comp**ddp_comp;
> > > > +struct drm_crtc_state*old_crtc_state;
> > > > };
> > > >
> > > > struct mtk_crtc_state {
> > > > @@ -362,6 +363,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
> > > > static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> > > > {
> > > > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > > > +struct drm_atomic_state *atomic_state = mtk_crtc->old_crtc_state->state;
> > > > struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
> > > > struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> > > > unsigned int i;
> > > > @@ -399,6 +401,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
> > > > plane_state->pending.config = false;
> > > > }
> > > > mtk_crtc->pending_planes = false;
> > > > +mtk_atomic_state_put_queue(atomic_state);
> > > > }
> > > > }
> > > >
> > > > @@ -494,6 +497,7 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
> > > > static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> > > > struct drm_crtc_state *old_crtc_state)
> > > > {
> > > > +struct drm_atomic_state *old_atomic_state = old_crtc_state->state;
> > > > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > > > struct mtk_drm_private *priv = crtc->dev->dev_private;
> > > > unsigned int pending_planes = 0;
> > > > @@ -512,8 +516,11 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> > > > pending_planes |= BIT(i);
> > > > }
> > > > }
> > > > -if (pending_planes)
> > > > +if (pending_planes) {
> > > > mtk_crtc->pending_planes = true;
> > > > +drm_atomic_state_get(old_atomic_state);
> > > > +mtk_crtc->old_crtc_state = old_crtc_state;
> > > > +}
> > > > if (crtc->state->color_mgmt_changed)
> > > > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
> > > > mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > > index 6588dc6dd5e3..6c68283b6124 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > > > @@ -115,10 +115,85 @@ static int mtk_atomic_commit(struct drm_device *drm,
> > > > return 0;
> > > > }
> > > >
> > > > +struct mtk_atomic_state {
> > > > +struct drm_atomic_state base;
> > > > +struct list_head list;
> > > > +};
> > > > +
> > > > +static inline struct mtk_atomic_state *to_mtk_state(struct drm_atomic_state *s)
> > > > +{
> > > > +return container_of(s, struct mtk_atomic_state, base);
> > > > +}
> > > > +
> > > > +void mtk_atomic_state_put_queue(struct drm_atomic_state *state)
> > > > +{
> > > > +struct drm_device *drm = state->dev;
> > > > +struct mtk_drm_private *mtk_drm = drm->dev_private;
> > > > +struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> > > > +unsigned long flags;
> > > > +
> > > > +spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> > > > +list_add_tail(&mtk_state->list, &mtk_drm->unreference.list);
> > > > +spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> > > > +
> > > > +schedule_work(&mtk_drm->unreference.work);
> > > > +}
> > > > +
> > > > +static void mtk_unreference_work(struct work_struct *work)
> > > > +{
> > > > +struct mtk_drm_private *mtk_drm = container_of(work,
> > > > +struct mtk_drm_private, unreference.work);
> > > > +unsigned long flags;
> > > > +struct mtk_atomic_state *state, *tmp;
> > > > +
> > > > +/*
> > > > + * framebuffers cannot be unreferenced in atomic context.
> > > > + * Therefore, only hold the spinlock when iterating unreference_list,
> > > > + * and drop it when doing the unreference.
> > > > + */
> > > > +spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> > > > +list_for_each_entry_safe(state, tmp, &mtk_drm->unreference.list, list) {
> > > > +list_del(&state->list);
> > > > +spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> > > > +drm_atomic_state_put(&state->base);
> > > > +spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
> > > > +}
> > > > +spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
> > > > +}
> > > > +
> > > > +static struct drm_atomic_state *
> > > > +mtk_drm_atomic_state_alloc(struct drm_device *dev)
> > > > +{
> > > > +struct mtk_atomic_state *mtk_state;
> > > > +
> > > > +mtk_state = kzalloc(sizeof(*mtk_state), GFP_KERNEL);
> > > > +if (!mtk_state)
> > > > +return NULL;
> > > > +
> > > > +if (drm_atomic_state_init(dev, &mtk_state->base) < 0) {
> > > > +kfree(mtk_state);
> > > > +return NULL;
> > > > +}
> > > > +
> > > > +INIT_LIST_HEAD(&mtk_state->list);
> > > > +
> > > > +return &mtk_state->base;
> > > > +}
> > > > +
> > > > +static void mtk_drm_atomic_state_free(struct drm_atomic_state *state)
> > > > +{
> > > > +struct mtk_atomic_state *mtk_state = to_mtk_state(state);
> > > > +
> > > > +drm_atomic_state_default_release(state);
> > > > +kfree(mtk_state);
> > > > +}
> > > > +
> > > > static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
> > > > .fb_create = mtk_drm_mode_fb_create,
> > > > .atomic_check = drm_atomic_helper_check,
> > > > .atomic_commit = mtk_atomic_commit,
> > > > +.atomic_state_alloc = mtk_drm_atomic_state_alloc,
> > > > +.atomic_state_free = mtk_drm_atomic_state_free
> > > > };
> > > >
> > > > static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
> > > > @@ -337,6 +412,10 @@ static int mtk_drm_kms_init(struct drm_device *drm)
> > > > drm_kms_helper_poll_init(drm);
> > > > drm_mode_config_reset(drm);
> > > >
> > > > +INIT_WORK(&private->unreference.work, mtk_unreference_work);
> > > > +INIT_LIST_HEAD(&private->unreference.list);
> > > > +spin_lock_init(&private->unreference.lock);
> > > > +
> > > > return 0;
> > > >
> > > > err_unset_dma_parms:
> > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > > index b6a82728d563..c37d835cf949 100644
> > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> > > > @@ -55,6 +55,13 @@ struct mtk_drm_private {
> > > >
> > > > struct drm_atomic_state *suspend_state;
> > > >
> > > > +struct {
> > > > +struct work_structwork;
> > > > +struct list_headlist;
> > > > +/* lock for unreference list */
> > > > +spinlock_tlock;
> > > > +} unreference;
> > > > +
> > > > bool dma_parms_allocated;
> > > > };
> > > >
> > > > @@ -66,4 +73,6 @@ extern struct platform_driver mtk_dpi_driver;
> > > > extern struct platform_driver mtk_dsi_driver;
> > > > extern struct platform_driver mtk_mipi_tx_driver;
> > > >
> > > > +void mtk_atomic_state_put_queue(struct drm_atomic_state *state);
> > > > +
> > > > #endif /* MTK_DRM_DRV_H */
> > > > --
> > > > 2.18.0
> > >
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-11-28 2:34 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26 6:29 [PATCH 0/7] drm/mediatek: fix cursor issue and apply CMDQ in Bibby Hsieh
2019-11-26 6:29 ` [PATCH 1/7] drm/mediatek: fix atomic_state reference counting Bibby Hsieh
2019-11-26 8:49 ` Daniel Vetter
2019-11-27 1:41 ` Bibby Hsieh
2019-11-27 9:56 ` Daniel Vetter
2019-11-28 2:34 ` Bibby Hsieh
2019-11-26 6:29 ` [PATCH 2/7] drm/mediatek: put "event" in critical section Bibby Hsieh
2019-11-26 6:29 ` [PATCH 3/7] drm/mediatek: use DRM core's atomic commit helper Bibby Hsieh
2019-11-26 6:29 ` [PATCH 4/7] drm/mediatek: handle events when enabling/disabling crtc Bibby Hsieh
2019-11-26 6:29 ` [PATCH 5/7] drm/mediatek: update cursors by using async atomic update Bibby Hsieh
2019-11-26 6:29 ` [PATCH 6/7] drm/mediatek: support CMDQ interface in ddp component Bibby Hsieh
2019-11-26 6:29 ` [PATCH 7/7] drm/mediatek: apply CMDQ control flow Bibby Hsieh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).