These patches based on 4.7-rc1 to clean up unused function & variable and use drm core function instead.
The following patches are needed to cleanly apply on top of v4.7-rc1: - https://patchwork.kernel.org/patch/8044001/ (drm: Deal with rotation in drm_plane_helper_check_update()) - https://patchwork.kernel.org/patch/9248373/ (drm: Warn about negative sizes when calculating scale factor) - https://patchwork.kernel.org/patch/9248371/ (drm: Store clipped src/dst coordinatee in drm_plane_state) - https://patchwork.kernel.org/patch/9248363/ (drm/plane-helper: Add drm_plane_helper_check_state()) - https://patchwork.kernel.org/patch/9248361/ (drm/mediatek: Use drm_plane_helper_check_state())
Bibby Hsieh (2): drm/mediatek: Use drm_atomic destroy_state helpers drm/mediatek: Fix mtk_atomic_complete for runtime_pm
Daniel Kurtz (5): drm/mediatek: Remove mtk_drm_crtc_check_flush drm/mediatek: plane: Remove plane zpos/index drm/mediatek: Remove mtk_drm_plane drm/mediatek: plane: Merge mtk_plane_enable into mtk_plane_atomic_update drm/mediatek: plane: Use FB's format's cpp to compute x offset
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 21 ++++---- drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.c | 17 ++++++- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 80 +++++++++++------------------- drivers/gpu/drm/mediatek/mtk_drm_plane.h | 15 +----- 5 files changed, 56 insertions(+), 78 deletions(-)
From: Daniel Kurtz djkurtz@chromium.org
This function no longer exists.
Signed-off-by: Daniel Kurtz djkurtz@chromium.org Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com --- drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h index 81e5566..4d32cf1 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h @@ -22,7 +22,6 @@
int mtk_drm_crtc_enable_vblank(struct drm_device *drm, unsigned int pipe); void mtk_drm_crtc_disable_vblank(struct drm_device *drm, unsigned int pipe); -void mtk_drm_crtc_check_flush(struct drm_crtc *crtc); void mtk_drm_crtc_commit(struct drm_crtc *crtc); void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *ovl); int mtk_drm_crtc_create(struct drm_device *drm_dev,
From: Daniel Kurtz djkurtz@chromium.org
It is not actually useful to a mtk plane to know its zpos/index, so just remove this field.
This let's us completely remove struct mtk_drm_plane in a follow up patch.
Signed-off-by: Daniel Kurtz djkurtz@chromium.org Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 4 +--- drivers/gpu/drm/mediatek/mtk_drm_plane.h | 4 +--- 3 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 24aa3ba..18211ab 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -559,7 +559,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, (zpos == 1) ? DRM_PLANE_TYPE_CURSOR : DRM_PLANE_TYPE_OVERLAY; ret = mtk_plane_init(drm_dev, &mtk_crtc->planes[zpos], - BIT(pipe), type, zpos); + BIT(pipe), type); if (ret) goto unprepare; } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 093db07..d28db57 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -189,8 +189,7 @@ static const struct drm_plane_helper_funcs mtk_plane_helper_funcs = { };
int mtk_plane_init(struct drm_device *dev, struct mtk_drm_plane *mtk_plane, - unsigned long possible_crtcs, enum drm_plane_type type, - unsigned int zpos) + unsigned long possible_crtcs, enum drm_plane_type type) { int err;
@@ -203,7 +202,6 @@ int mtk_plane_init(struct drm_device *dev, struct mtk_drm_plane *mtk_plane, }
drm_plane_helper_add(&mtk_plane->base, &mtk_plane_helper_funcs); - mtk_plane->idx = zpos;
return 0; } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h index 72a7b3e..74dbeda 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h @@ -20,7 +20,6 @@
struct mtk_drm_plane { struct drm_plane base; - unsigned int idx; };
struct mtk_plane_pending_state { @@ -53,7 +52,6 @@ to_mtk_plane_state(struct drm_plane_state *state) }
int mtk_plane_init(struct drm_device *dev, struct mtk_drm_plane *mtk_plane, - unsigned long possible_crtcs, enum drm_plane_type type, - unsigned int zpos); + unsigned long possible_crtcs, enum drm_plane_type type);
#endif
Hi, Bibby:
On Fri, 2016-07-29 at 17:04 +0800, Bibby Hsieh wrote:
From: Daniel Kurtz djkurtz@chromium.org
It is not actually useful to a mtk plane to know its zpos/index, so just remove this field.
This let's us completely remove struct mtk_drm_plane in a follow up patch.
'let's us'? My English is not as good as native English speaker. Otherwise, this patch looks good to me.
Signed-off-by: Daniel Kurtz djkurtz@chromium.org Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 4 +--- drivers/gpu/drm/mediatek/mtk_drm_plane.h | 4 +--- 3 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 24aa3ba..18211ab 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -559,7 +559,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, (zpos == 1) ? DRM_PLANE_TYPE_CURSOR : DRM_PLANE_TYPE_OVERLAY; ret = mtk_plane_init(drm_dev, &mtk_crtc->planes[zpos],
BIT(pipe), type, zpos);
BIT(pipe), type);diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 093db07..d28db57 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -189,8 +189,7 @@ static const struct drm_plane_helper_funcs mtk_plane_helper_funcs = { };
int mtk_plane_init(struct drm_device *dev, struct mtk_drm_plane *mtk_plane,
unsigned long possible_crtcs, enum drm_plane_type type,unsigned int zpos)
unsigned long possible_crtcs, enum drm_plane_type type){ int err;
@@ -203,7 +202,6 @@ int mtk_plane_init(struct drm_device *dev, struct mtk_drm_plane *mtk_plane, }
drm_plane_helper_add(&mtk_plane->base, &mtk_plane_helper_funcs);
mtk_plane->idx = zpos;
return 0;
} diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h index 72a7b3e..74dbeda 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h @@ -20,7 +20,6 @@
struct mtk_drm_plane { struct drm_plane base;
- unsigned int idx;
};
struct mtk_plane_pending_state { @@ -53,7 +52,6 @@ to_mtk_plane_state(struct drm_plane_state *state) }
int mtk_plane_init(struct drm_device *dev, struct mtk_drm_plane *mtk_plane,
unsigned long possible_crtcs, enum drm_plane_type type,unsigned int zpos);
unsigned long possible_crtcs, enum drm_plane_type type);#endif
Regards, CK
From: Daniel Kurtz djkurtz@chromium.org
Now that mtk_drm_plane just contains its base struct drm_plane, we can just remove it and use struct drm_plane everywhere.
Signed-off-by: Daniel Kurtz djkurtz@chromium.org Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 16 ++++++++-------- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 10 ++++------ drivers/gpu/drm/mediatek/mtk_drm_plane.h | 11 +---------- 3 files changed, 13 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 18211ab..d6fbefa 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -31,7 +31,7 @@ * struct mtk_drm_crtc - MediaTek specific crtc structure. * @base: crtc object. * @enabled: records whether crtc_enable succeeded - * @planes: array of 4 mtk_drm_plane structures, one for each overlay plane + * @planes: array of 4 drm_plane structures, one for each overlay plane * @pending_planes: whether any plane has pending changes to be applied * @config_regs: memory mapped mmsys configuration register space * @mutex: handle to one of the ten disp_mutex streams @@ -45,7 +45,7 @@ struct mtk_drm_crtc { bool pending_needs_vblank; struct drm_pending_vblank_event *event;
- struct mtk_drm_plane planes[OVL_LAYER_NR]; + struct drm_plane planes[OVL_LAYER_NR]; bool pending_planes;
void __iomem *config_regs; @@ -272,7 +272,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
/* Initially configure all planes */ for (i = 0; i < OVL_LAYER_NR; i++) { - struct drm_plane *plane = &mtk_crtc->planes[i].base; + struct drm_plane *plane = &mtk_crtc->planes[i]; struct mtk_plane_state *plane_state;
plane_state = to_mtk_plane_state(plane->state); @@ -351,7 +351,7 @@ static void mtk_drm_crtc_disable(struct drm_crtc *crtc)
/* Set all pending plane state to disabled */ for (i = 0; i < OVL_LAYER_NR; i++) { - struct drm_plane *plane = &mtk_crtc->planes[i].base; + struct drm_plane *plane = &mtk_crtc->planes[i]; struct mtk_plane_state *plane_state;
plane_state = to_mtk_plane_state(plane->state); @@ -397,7 +397,7 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc, if (mtk_crtc->event) mtk_crtc->pending_needs_vblank = true; for (i = 0; i < OVL_LAYER_NR; i++) { - struct drm_plane *plane = &mtk_crtc->planes[i].base; + struct drm_plane *plane = &mtk_crtc->planes[i]; struct mtk_plane_state *plane_state;
plane_state = to_mtk_plane_state(plane->state); @@ -471,7 +471,7 @@ void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *ovl)
if (mtk_crtc->pending_planes) { for (i = 0; i < OVL_LAYER_NR; i++) { - struct drm_plane *plane = &mtk_crtc->planes[i].base; + struct drm_plane *plane = &mtk_crtc->planes[i]; struct mtk_plane_state *plane_state;
plane_state = to_mtk_plane_state(plane->state); @@ -564,8 +564,8 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, goto unprepare; }
- ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, &mtk_crtc->planes[0].base, - &mtk_crtc->planes[1].base, pipe); + ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, &mtk_crtc->planes[0], + &mtk_crtc->planes[1], pipe); if (ret < 0) goto unprepare;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index d28db57..86b7aed 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -33,7 +33,6 @@ static const u32 formats[] = { static void mtk_plane_enable(struct drm_plane *plane, dma_addr_t addr) { - struct drm_plane *plane = &mtk_plane->base; struct mtk_plane_state *state = to_mtk_plane_state(plane->state); unsigned int pitch, format; bool enable; @@ -162,14 +161,13 @@ static void mtk_plane_atomic_update(struct drm_plane *plane, struct drm_crtc *crtc = state->base.crtc; struct drm_gem_object *gem; struct mtk_drm_gem_obj *mtk_gem; - struct mtk_drm_plane *mtk_plane = to_mtk_plane(plane);
if (!crtc) return;
gem = mtk_fb_get_gem_obj(state->base.fb); mtk_gem = to_mtk_gem_obj(gem); - mtk_plane_enable(mtk_plane, mtk_gem->dma_addr); + mtk_plane_enable(plane, mtk_gem->dma_addr); }
static void mtk_plane_atomic_disable(struct drm_plane *plane, @@ -188,12 +186,12 @@ static const struct drm_plane_helper_funcs mtk_plane_helper_funcs = { .atomic_disable = mtk_plane_atomic_disable, };
-int mtk_plane_init(struct drm_device *dev, struct mtk_drm_plane *mtk_plane, +int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane, unsigned long possible_crtcs, enum drm_plane_type type) { int err;
- err = drm_universal_plane_init(dev, &mtk_plane->base, possible_crtcs, + err = drm_universal_plane_init(dev, plane, possible_crtcs, &mtk_plane_funcs, formats, ARRAY_SIZE(formats), type, NULL); if (err) { @@ -201,7 +199,7 @@ int mtk_plane_init(struct drm_device *dev, struct mtk_drm_plane *mtk_plane, return err; }
- drm_plane_helper_add(&mtk_plane->base, &mtk_plane_helper_funcs); + drm_plane_helper_add(plane, &mtk_plane_helper_funcs);
return 0; } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h index 74dbeda..6a20b49 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h @@ -18,10 +18,6 @@ #include <drm/drm_crtc.h> #include <linux/types.h>
-struct mtk_drm_plane { - struct drm_plane base; -}; - struct mtk_plane_pending_state { bool config; bool enable; @@ -40,18 +36,13 @@ struct mtk_plane_state { struct mtk_plane_pending_state pending; };
-static inline struct mtk_drm_plane *to_mtk_plane(struct drm_plane *plane) -{ - return container_of(plane, struct mtk_drm_plane, base); -} - static inline struct mtk_plane_state * to_mtk_plane_state(struct drm_plane_state *state) { return container_of(state, struct mtk_plane_state, base); }
-int mtk_plane_init(struct drm_device *dev, struct mtk_drm_plane *mtk_plane, +int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane, unsigned long possible_crtcs, enum drm_plane_type type);
#endif
Use the core destroy_state helpers to destroy core state to ensure we don't leak if/when more fields get added later.
Signed-off-by: Daniel Kurtz djkurtz@chromium.org Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 3 +-- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index d6fbefa..733b2a3 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -112,8 +112,7 @@ static void mtk_drm_crtc_reset(struct drm_crtc *crtc) struct mtk_crtc_state *state;
if (crtc->state) { - if (crtc->state->mode_blob) - drm_property_unreference_blob(crtc->state->mode_blob); + __drm_atomic_helper_crtc_destroy_state(crtc->state);
state = to_mtk_crtc_state(crtc->state); memset(state, 0, sizeof(*state)); diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 86b7aed..17172ba 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -73,8 +73,7 @@ static void mtk_plane_reset(struct drm_plane *plane) struct mtk_plane_state *state;
if (plane->state) { - if (plane->state->fb) - drm_framebuffer_unreference(plane->state->fb); + __drm_atomic_helper_plane_destroy_state(plane->state);
state = to_mtk_plane_state(plane->state); memset(state, 0, sizeof(*state));
From: Daniel Kurtz djkurtz@chromium.org
The mtk_plane_enable is just called once by mtk_plane_atomic_update. So, merge mtk_plane_enable into mtk_plane_atomic_update.
While we are here, also clean up the function a bit by using an fb local variables.
Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com Signed-off-by: Daniel Kurtz djkurtz@chromium.org --- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 65 +++++++++++------------------- 1 file changed, 23 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 17172ba..b3ddb20 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -30,44 +30,6 @@ static const u32 formats[] = { DRM_FORMAT_RGB565, };
-static void mtk_plane_enable(struct drm_plane *plane, - dma_addr_t addr) -{ - struct mtk_plane_state *state = to_mtk_plane_state(plane->state); - unsigned int pitch, format; - bool enable; - - if (WARN_ON(!plane->state)) - return; - - enable = state->base.visible; - - if (WARN_ON(enable && !plane->state->fb)) - return; - - if (plane->state->fb) { - pitch = plane->state->fb->pitches[0]; - format = plane->state->fb->pixel_format; - } else { - pitch = 0; - format = DRM_FORMAT_RGBA8888; - } - - addr += (state->base.src.x1 >> 16) * 4; - addr += (state->base.src.y1 >> 16) * pitch; - - state->pending.enable = enable; - state->pending.pitch = pitch; - state->pending.format = format; - state->pending.addr = addr; - state->pending.x = state->base.dst.x1; - state->pending.y = state->base.dst.y1; - state->pending.width = drm_rect_width(&state->base.dst); - state->pending.height = drm_rect_height(&state->base.dst); - wmb(); /* Make sure the above parameters are set before update */ - state->pending.dirty = true; -} - static void mtk_plane_reset(struct drm_plane *plane) { struct mtk_plane_state *state; @@ -157,16 +119,35 @@ static void mtk_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { struct mtk_plane_state *state = to_mtk_plane_state(plane->state); - struct drm_crtc *crtc = state->base.crtc; + struct drm_crtc *crtc = plane->state->crtc; + struct drm_framebuffer *fb = plane->state->fb; struct drm_gem_object *gem; struct mtk_drm_gem_obj *mtk_gem; + unsigned int pitch, format; + dma_addr_t addr;
- if (!crtc) + if (!crtc || WARN_ON(!fb)) return;
- gem = mtk_fb_get_gem_obj(state->base.fb); + gem = mtk_fb_get_gem_obj(fb); mtk_gem = to_mtk_gem_obj(gem); - mtk_plane_enable(plane, mtk_gem->dma_addr); + addr = mtk_gem->dma_addr; + pitch = fb->pitches[0]; + format = fb->pixel_format; + + addr += (plane->state->src.x1 >> 16) * 4; + addr += (plane->state->src.y1 >> 16) * pitch; + + state->pending.enable = true; + state->pending.pitch = pitch; + state->pending.format = format; + state->pending.addr = addr; + state->pending.x = plane->state->dst.x1; + state->pending.y = plane->state->dst.y1; + state->pending.width = drm_rect_width(&plane->state->dst); + state->pending.height = drm_rect_height(&plane->state->dst); + wmb(); /* Make sure the above parameters are set before update */ + state->pending.dirty = true; }
static void mtk_plane_atomic_disable(struct drm_plane *plane,
On Fri, Jul 29, 2016 at 5:04 AM, Bibby Hsieh bibby.hsieh@mediatek.com wrote:
From: Daniel Kurtz djkurtz@chromium.org
The mtk_plane_enable is just called once by mtk_plane_atomic_update. So, merge mtk_plane_enable into mtk_plane_atomic_update.
While we are here, also clean up the function a bit by using an fb local variables.
Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com Signed-off-by: Daniel Kurtz djkurtz@chromium.org
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 65 +++++++++++------------------- 1 file changed, 23 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 17172ba..b3ddb20 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -30,44 +30,6 @@ static const u32 formats[] = { DRM_FORMAT_RGB565, };
-static void mtk_plane_enable(struct drm_plane *plane,
Hi Bibby, This doesn't merge cleanly.
It seems like upstream has:
static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane,
Whereas your version of "is based on some downstream version of "drm/mediatek: Use drm_plane_helper_check_state()" which changed this to struct drm_plane. Can you respin the set such that the mtk_drm_plane removal patch fixes this?
Your set should apply cleanly to https://cgit.freedesktop.org/~seanpaul/birch/?h=for-misc
Thanks,
Sean
dma_addr_t addr)-{
struct mtk_plane_state *state = to_mtk_plane_state(plane->state);unsigned int pitch, format;bool enable;if (WARN_ON(!plane->state))return;enable = state->base.visible;if (WARN_ON(enable && !plane->state->fb))return;if (plane->state->fb) {pitch = plane->state->fb->pitches[0];format = plane->state->fb->pixel_format;} else {pitch = 0;format = DRM_FORMAT_RGBA8888;}addr += (state->base.src.x1 >> 16) * 4;addr += (state->base.src.y1 >> 16) * pitch;state->pending.enable = enable;state->pending.pitch = pitch;state->pending.format = format;state->pending.addr = addr;state->pending.x = state->base.dst.x1;state->pending.y = state->base.dst.y1;state->pending.width = drm_rect_width(&state->base.dst);state->pending.height = drm_rect_height(&state->base.dst);wmb(); /* Make sure the above parameters are set before update */state->pending.dirty = true;-}
static void mtk_plane_reset(struct drm_plane *plane) { struct mtk_plane_state *state; @@ -157,16 +119,35 @@ static void mtk_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { struct mtk_plane_state *state = to_mtk_plane_state(plane->state);
struct drm_crtc *crtc = state->base.crtc;
struct drm_crtc *crtc = plane->state->crtc;struct drm_framebuffer *fb = plane->state->fb; struct drm_gem_object *gem; struct mtk_drm_gem_obj *mtk_gem;unsigned int pitch, format;dma_addr_t addr;
if (!crtc)
if (!crtc || WARN_ON(!fb)) return;
gem = mtk_fb_get_gem_obj(state->base.fb);
gem = mtk_fb_get_gem_obj(fb); mtk_gem = to_mtk_gem_obj(gem);
mtk_plane_enable(plane, mtk_gem->dma_addr);
addr = mtk_gem->dma_addr;pitch = fb->pitches[0];format = fb->pixel_format;addr += (plane->state->src.x1 >> 16) * 4;addr += (plane->state->src.y1 >> 16) * pitch;state->pending.enable = true;state->pending.pitch = pitch;state->pending.format = format;state->pending.addr = addr;state->pending.x = plane->state->dst.x1;state->pending.y = plane->state->dst.y1;state->pending.width = drm_rect_width(&plane->state->dst);state->pending.height = drm_rect_height(&plane->state->dst);wmb(); /* Make sure the above parameters are set before update */state->pending.dirty = true;}
static void mtk_plane_atomic_disable(struct drm_plane *plane,
1.7.9.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Aug 2, 2016 at 1:02 PM, Sean Paul seanpaul@chromium.org wrote:
On Fri, Jul 29, 2016 at 5:04 AM, Bibby Hsieh bibby.hsieh@mediatek.com wrote:
From: Daniel Kurtz djkurtz@chromium.org
The mtk_plane_enable is just called once by mtk_plane_atomic_update. So, merge mtk_plane_enable into mtk_plane_atomic_update.
While we are here, also clean up the function a bit by using an fb local variables.
Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com Signed-off-by: Daniel Kurtz djkurtz@chromium.org
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 65 +++++++++++------------------- 1 file changed, 23 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 17172ba..b3ddb20 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -30,44 +30,6 @@ static const u32 formats[] = { DRM_FORMAT_RGB565, };
-static void mtk_plane_enable(struct drm_plane *plane,
Hi Bibby, This doesn't merge cleanly.
It seems like upstream has:
static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane,
Whereas your version of "is based on some downstream version of "drm/mediatek: Use drm_plane_helper_check_state()" which changed this to struct drm_plane. Can you respin the set such that the mtk_drm_plane removal patch fixes this?
Let me try that again, looks like I mangled my cut/paste:
Whereas your version of "drm/mediatek: Use drm_plane_helper_check_state()" is based on some downstream version which changed this to struct drm_plane. Can you respin the set such that the mtk_drm_plane removal patch fixes this?
Sean
Your set should apply cleanly to https://cgit.freedesktop.org/~seanpaul/birch/?h=for-misc
Thanks,
Sean
dma_addr_t addr)-{
struct mtk_plane_state *state = to_mtk_plane_state(plane->state);unsigned int pitch, format;bool enable;if (WARN_ON(!plane->state))return;enable = state->base.visible;if (WARN_ON(enable && !plane->state->fb))return;if (plane->state->fb) {pitch = plane->state->fb->pitches[0];format = plane->state->fb->pixel_format;} else {pitch = 0;format = DRM_FORMAT_RGBA8888;}addr += (state->base.src.x1 >> 16) * 4;addr += (state->base.src.y1 >> 16) * pitch;state->pending.enable = enable;state->pending.pitch = pitch;state->pending.format = format;state->pending.addr = addr;state->pending.x = state->base.dst.x1;state->pending.y = state->base.dst.y1;state->pending.width = drm_rect_width(&state->base.dst);state->pending.height = drm_rect_height(&state->base.dst);wmb(); /* Make sure the above parameters are set before update */state->pending.dirty = true;-}
static void mtk_plane_reset(struct drm_plane *plane) { struct mtk_plane_state *state; @@ -157,16 +119,35 @@ static void mtk_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { struct mtk_plane_state *state = to_mtk_plane_state(plane->state);
struct drm_crtc *crtc = state->base.crtc;
struct drm_crtc *crtc = plane->state->crtc;struct drm_framebuffer *fb = plane->state->fb; struct drm_gem_object *gem; struct mtk_drm_gem_obj *mtk_gem;unsigned int pitch, format;dma_addr_t addr;
if (!crtc)
if (!crtc || WARN_ON(!fb)) return;
gem = mtk_fb_get_gem_obj(state->base.fb);
gem = mtk_fb_get_gem_obj(fb); mtk_gem = to_mtk_gem_obj(gem);
mtk_plane_enable(plane, mtk_gem->dma_addr);
addr = mtk_gem->dma_addr;pitch = fb->pitches[0];format = fb->pixel_format;addr += (plane->state->src.x1 >> 16) * 4;addr += (plane->state->src.y1 >> 16) * pitch;state->pending.enable = true;state->pending.pitch = pitch;state->pending.format = format;state->pending.addr = addr;state->pending.x = plane->state->dst.x1;state->pending.y = plane->state->dst.y1;state->pending.width = drm_rect_width(&plane->state->dst);state->pending.height = drm_rect_height(&plane->state->dst);wmb(); /* Make sure the above parameters are set before update */state->pending.dirty = true;}
static void mtk_plane_atomic_disable(struct drm_plane *plane,
1.7.9.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 2016-08-02 at 13:04 -0400, Sean Paul wrote:
On Tue, Aug 2, 2016 at 1:02 PM, Sean Paul seanpaul@chromium.org wrote:
On Fri, Jul 29, 2016 at 5:04 AM, Bibby Hsieh bibby.hsieh@mediatek.com wrote:
From: Daniel Kurtz djkurtz@chromium.org
The mtk_plane_enable is just called once by mtk_plane_atomic_update. So, merge mtk_plane_enable into mtk_plane_atomic_update.
While we are here, also clean up the function a bit by using an fb local variables.
Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com Signed-off-by: Daniel Kurtz djkurtz@chromium.org
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 65 +++++++++++------------------- 1 file changed, 23 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index 17172ba..b3ddb20 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -30,44 +30,6 @@ static const u32 formats[] = { DRM_FORMAT_RGB565, };
-static void mtk_plane_enable(struct drm_plane *plane,
Hi Bibby, This doesn't merge cleanly.
It seems like upstream has:
static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane,
Whereas your version of "is based on some downstream version of "drm/mediatek: Use drm_plane_helper_check_state()" which changed this to struct drm_plane. Can you respin the set such that the mtk_drm_plane removal patch fixes this?
Let me try that again, looks like I mangled my cut/paste:
Whereas your version of "drm/mediatek: Use drm_plane_helper_check_state()" is based on some downstream version which changed this to struct drm_plane. Can you respin the set such that the mtk_drm_plane removal patch fixes this?
Sorry about the mistake, I am going to fix that as soon as possible, thanks for your review.
Bibby
Sean
Your set should apply cleanly to https://cgit.freedesktop.org/~seanpaul/birch/?h=for-misc
Thanks,
Sean
dma_addr_t addr)-{
struct mtk_plane_state *state = to_mtk_plane_state(plane->state);unsigned int pitch, format;bool enable;if (WARN_ON(!plane->state))return;enable = state->base.visible;if (WARN_ON(enable && !plane->state->fb))return;if (plane->state->fb) {pitch = plane->state->fb->pitches[0];format = plane->state->fb->pixel_format;} else {pitch = 0;format = DRM_FORMAT_RGBA8888;}addr += (state->base.src.x1 >> 16) * 4;addr += (state->base.src.y1 >> 16) * pitch;state->pending.enable = enable;state->pending.pitch = pitch;state->pending.format = format;state->pending.addr = addr;state->pending.x = state->base.dst.x1;state->pending.y = state->base.dst.y1;state->pending.width = drm_rect_width(&state->base.dst);state->pending.height = drm_rect_height(&state->base.dst);wmb(); /* Make sure the above parameters are set before update */state->pending.dirty = true;-}
static void mtk_plane_reset(struct drm_plane *plane) { struct mtk_plane_state *state; @@ -157,16 +119,35 @@ static void mtk_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { struct mtk_plane_state *state = to_mtk_plane_state(plane->state);
struct drm_crtc *crtc = state->base.crtc;
struct drm_crtc *crtc = plane->state->crtc;struct drm_framebuffer *fb = plane->state->fb; struct drm_gem_object *gem; struct mtk_drm_gem_obj *mtk_gem;unsigned int pitch, format;dma_addr_t addr;
if (!crtc)
if (!crtc || WARN_ON(!fb)) return;
gem = mtk_fb_get_gem_obj(state->base.fb);
gem = mtk_fb_get_gem_obj(fb); mtk_gem = to_mtk_gem_obj(gem);
mtk_plane_enable(plane, mtk_gem->dma_addr);
addr = mtk_gem->dma_addr;pitch = fb->pitches[0];format = fb->pixel_format;addr += (plane->state->src.x1 >> 16) * 4;addr += (plane->state->src.y1 >> 16) * pitch;state->pending.enable = true;state->pending.pitch = pitch;state->pending.format = format;state->pending.addr = addr;state->pending.x = plane->state->dst.x1;state->pending.y = plane->state->dst.y1;state->pending.width = drm_rect_width(&plane->state->dst);state->pending.height = drm_rect_height(&plane->state->dst);wmb(); /* Make sure the above parameters are set before update */state->pending.dirty = true;}
static void mtk_plane_atomic_disable(struct drm_plane *plane,
1.7.9.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Daniel Kurtz djkurtz@chromium.org
Use the framebuffer's format to compute its cpp, and use it when calculating the address shift value.
Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com --- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index b3ddb20..c461a23 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -135,7 +135,7 @@ static void mtk_plane_atomic_update(struct drm_plane *plane, pitch = fb->pitches[0]; format = fb->pixel_format;
- addr += (plane->state->src.x1 >> 16) * 4; + addr += (plane->state->src.x1 >> 16) * drm_format_plane_cpp(format, 0); addr += (plane->state->src.y1 >> 16) * pitch;
state->pending.enable = true;
To properly implement atomic w/ runtime pm, we move drm_atomic_helper_commit_modeset_enables() above drm_atomic_helper_commit_planes() to ensure CRTCs are enabled before modifying plane registers, and set active_only to true to filter out plane update notifications when the CRTC is disabled.
According to the document from linux kernel: Set the active_only parameters to true in order not to receive plane update notifications related to a disabled CRTC. This avoids the need to manually ignore plane updates in driver code when the driver and/or hardware can't or just don't need to deal with updates on disabled CRTCs, for example when supporting runtime PM.
Signed-off-by: Bibby Hsieh bibby.hsieh@mediatek.com Signed-off-by: Daniel Kurtz djkurtz@chromium.org --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index b1223d5..507392a 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -61,10 +61,25 @@ static void mtk_atomic_complete(struct mtk_drm_private *private,
mtk_atomic_wait_for_fences(state);
+ /* + * 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, true); + * + * See the kerneldoc entries for these three functions for more details. + */ drm_atomic_helper_commit_modeset_disables(drm, state); - drm_atomic_helper_commit_planes(drm, state, false); drm_atomic_helper_commit_modeset_enables(drm, state); + drm_atomic_helper_commit_planes(drm, state, true); + drm_atomic_helper_wait_for_vblanks(drm, state); + drm_atomic_helper_cleanup_planes(drm, state); drm_atomic_state_free(state); }
On Fri, Jul 29, 2016 at 5:04 AM, Bibby Hsieh bibby.hsieh@mediatek.com wrote:
These patches based on 4.7-rc1 to clean up unused function & variable and use drm core function instead.
The following patches are needed to cleanly apply on top of v4.7-rc1:
- https://patchwork.kernel.org/patch/8044001/ (drm: Deal with rotation in drm_plane_helper_check_update())
- https://patchwork.kernel.org/patch/9248373/ (drm: Warn about negative sizes when calculating scale factor)
- https://patchwork.kernel.org/patch/9248371/ (drm: Store clipped src/dst coordinatee in drm_plane_state)
- https://patchwork.kernel.org/patch/9248363/ (drm/plane-helper: Add drm_plane_helper_check_state())
- https://patchwork.kernel.org/patch/9248361/ (drm/mediatek: Use drm_plane_helper_check_state())
Bibby Hsieh (2): drm/mediatek: Use drm_atomic destroy_state helpers drm/mediatek: Fix mtk_atomic_complete for runtime_pm
Daniel Kurtz (5): drm/mediatek: Remove mtk_drm_crtc_check_flush drm/mediatek: plane: Remove plane zpos/index drm/mediatek: Remove mtk_drm_plane drm/mediatek: plane: Merge mtk_plane_enable into mtk_plane_atomic_update drm/mediatek: plane: Use FB's format's cpp to compute x offset
The whole set looks good to me.
Reviewed-by: Sean Paul seanpaul@chromium.org
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 21 ++++---- drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.c | 17 ++++++- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 80 +++++++++++------------------- drivers/gpu/drm/mediatek/mtk_drm_plane.h | 15 +----- 5 files changed, 56 insertions(+), 78 deletions(-)
-- 1.7.9.5
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Bibby Hsieh -
CK Hu -
Sean Paul