linux-kernel - Re: [PATCH v1, 1/2] drm/mediatek: Fixup external display black screen issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1575352101.2457.8.camel@mtksdaap41>
Date:   Tue, 3 Dec 2019 13:48:21 +0800
From:   CK Hu <ck.hu@...iatek.com>
To:     <yongqiang.niu@...iatek.com>
CC:     Philipp Zabel <p.zabel@...gutronix.de>,
        Rob Herring <robh+dt@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        "David Airlie" <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        Mark Rutland <mark.rutland@....com>,
        <dri-devel@...ts.freedesktop.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH v1, 1/2] drm/mediatek: Fixup external display black
 screen issue

Hi, Yongqiang:

On Wed, 2019-11-27 at 09:17 +0800, yongqiang.niu@...iatek.com wrote:
> From: Yongqiang Niu <yongqiang.niu@...iatek.com>
> 
> Problem:
> overlay hangup when external display hotplut test
> 
> Fix:
> disable overlay when crtc disable

I think you do two things in this patch. The first is to config layer
before component start, and the second is disable layer when crtc
disable. So separate to two patches.

> 
> Signed-off-by: Yongqiang Niu <yongqiang.niu@...iatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 39 +++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 4fb346c..7eca02f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -369,6 +369,20 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
>  	mtk_disp_mutex_add_comp(mtk_crtc->mutex, mtk_crtc->ddp_comp[i]->id);
>  	mtk_disp_mutex_enable(mtk_crtc->mutex);
>  
> +	/* Initially configure all planes */
> +	for (i = 0; i < mtk_crtc->layer_nr; i++) {
> +		struct drm_plane *plane = &mtk_crtc->planes[i];
> +		struct mtk_plane_state *plane_state;
> +		struct mtk_ddp_comp *comp;
> +		unsigned int local_layer;
> +
> +		plane_state = to_mtk_plane_state(plane->state);
> +		comp = mtk_drm_ddp_comp_for_plane(crtc, plane, &local_layer);
> +		if (comp)
> +			mtk_ddp_comp_layer_config(comp, local_layer,
> +						  plane_state, NULL);
> +	}
> +
>  	for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
>  		struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[i];
>  		enum mtk_ddp_comp_id prev;
> @@ -385,20 +399,6 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
>  		mtk_ddp_comp_start(comp);
>  	}
>  
> -	/* Initially configure all planes */
> -	for (i = 0; i < mtk_crtc->layer_nr; i++) {
> -		struct drm_plane *plane = &mtk_crtc->planes[i];
> -		struct mtk_plane_state *plane_state;
> -		struct mtk_ddp_comp *comp;
> -		unsigned int local_layer;
> -
> -		plane_state = to_mtk_plane_state(plane->state);
> -		comp = mtk_drm_ddp_comp_for_plane(crtc, plane, &local_layer);
> -		if (comp)
> -			mtk_ddp_comp_layer_config(comp, local_layer,
> -						  plane_state, NULL);
> -	}
> -
>  	return 0;
>  
>  err_mutex_unprepare:
> @@ -607,10 +607,21 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc *crtc,
>  	for (i = 0; i < mtk_crtc->layer_nr; i++) {
>  		struct drm_plane *plane = &mtk_crtc->planes[i];
>  		struct mtk_plane_state *plane_state;
> +		struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
> +		unsigned int comp_layer_nr = mtk_ddp_comp_layer_nr(comp);
> +		unsigned int local_layer;
>  
>  		plane_state = to_mtk_plane_state(plane->state);
>  		plane_state->pending.enable = false;
>  		plane_state->pending.config = true;
> +
> +		if (i >= comp_layer_nr) {
> +			comp = mtk_crtc->ddp_comp[1];
> +			local_layer = i - comp_layer_nr;
> +		} else
> +			local_layer = i;
> +		mtk_ddp_comp_layer_config(comp, local_layer,
> +					  plane_state, NULL);

I'm confused with this part. The design of this loop is to set
plane_state->pending.enable = false and wait for irq handler to write
register. Why do you directly write register?

Regards,
CK

>  	}
>  	mtk_crtc->pending_planes = true;
>