Re: [PATCH v1 2/6] drm/mediatek: use DRM core's atomic commit helper

CK Hu
2019-12-02T06:17:33Z

Hi, Bibby:

On Thu, 2019-11-28 at 10:42 +0800, Bibby Hsieh wrote:
> The DRM core atomic helper now supports asynchronous commits natively.
> The custom drm implementation isn't needed anymore, remove it.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

Regards,
CK

>
> Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 86 ++------------------------
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 7 ---
> 2 files changed, 5 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 6588dc6dd5e3..16e5771d182e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -36,89 +36,14 @@
> #define DRIVER_MAJOR 1
> #define DRIVER_MINOR 0
>
> -static void mtk_atomic_schedule(struct mtk_drm_private *private,
> - struct drm_atomic_state *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;
> -
> - 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_PLANE_COMMIT_ACTIVE_ONLY);
> -
> - drm_atomic_helper_wait_for_vblanks(drm, state);
> -
> - drm_atomic_helper_cleanup_planes(drm, state);
> - drm_atomic_state_put(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 = drm_atomic_helper_commit_tail_rpm,
> +};
>
> 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,
> };
>
> static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
> @@ -265,6 +190,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)
> @@ -540,8 +466,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 b6a82728d563..9f4ce60174f6 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;
>
> bool dma_parms_allocated;