linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
@ 2021-07-10 13:24 Frank Wunderlich
  2021-07-12 11:50 ` Dafna Hirschfeld
  0 siblings, 1 reply; 8+ messages in thread
From: Frank Wunderlich @ 2021-07-10 13:24 UTC (permalink / raw)
  To: linux-mediatek
  Cc: Frank Wunderlich, Matthias Brugger, CK Hu, linux-arm-kernel,
	linux-kernel, Enric Balletbo Serra, Dafna Hirschfeld,
	David Airlie, dri-devel, Daniel Vetter

From: Frank Wunderlich <frank-w@public-files.de>

HDMI output was broken on mt7623/BPI-R2 in 5.13 because function for
special output selection (mtk_mmsys_ddp_sout_sel) was dropped.
This function wrote 3 registers at one time and so it is not compatible
with the array-approach.

I re-added the old function and call this after the default routing table
was applied. This default routing table can still be used as the only
connection handled by mmsys on BPI-R2 is OVL0->RDMA0 and this is already
present in the default routing table

Fixes: 440147639ac7 ("soc: mediatek: mmsys: Use an array for setting the routing registers")
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 drivers/soc/mediatek/mtk-mmsys.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index 080660ef11bf..f91b7fdd417a 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -57,6 +57,25 @@ struct mtk_mmsys {
 	const struct mtk_mmsys_driver_data *data;
 };
 
+static void mtk_mmsys_ddp_sout_sel(struct device *dev,
+				   enum mtk_ddp_comp_id cur,
+				   enum mtk_ddp_comp_id next)
+{
+	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
+
+	if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
+		writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
+			       mmsys->regs + DISP_REG_CONFIG_OUT_SEL);
+	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
+		writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI,
+			       mmsys->regs + DISP_REG_CONFIG_OUT_SEL);
+		writel_relaxed(DSI_SEL_IN_RDMA,
+			       mmsys->regs + DISP_REG_CONFIG_DSI_SEL);
+		writel_relaxed(DPI_SEL_IN_BLS,
+			       mmsys->regs + DISP_REG_CONFIG_DPI_SEL);
+	}
+}
+
 void mtk_mmsys_ddp_connect(struct device *dev,
 			   enum mtk_ddp_comp_id cur,
 			   enum mtk_ddp_comp_id next)
@@ -71,6 +90,8 @@ void mtk_mmsys_ddp_connect(struct device *dev,
 			reg = readl_relaxed(mmsys->regs + routes[i].addr) | routes[i].val;
 			writel_relaxed(reg, mmsys->regs + routes[i].addr);
 		}
+
+	mtk_mmsys_ddp_sout_sel(dev, cur, next);
 }
 EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect);
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
  2021-07-10 13:24 [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2 Frank Wunderlich
@ 2021-07-12 11:50 ` Dafna Hirschfeld
  2021-07-12 12:35   ` Aw: " Frank Wunderlich
  2021-07-12 17:16   ` Frank Wunderlich
  0 siblings, 2 replies; 8+ messages in thread
From: Dafna Hirschfeld @ 2021-07-12 11:50 UTC (permalink / raw)
  To: Frank Wunderlich, linux-mediatek
  Cc: Frank Wunderlich, Matthias Brugger, CK Hu, linux-arm-kernel,
	linux-kernel, Enric Balletbo Serra, David Airlie, dri-devel,
	Daniel Vetter

Hi

On 10.07.21 15:24, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> HDMI output was broken on mt7623/BPI-R2 in 5.13 because function for
> special output selection (mtk_mmsys_ddp_sout_sel) was dropped.
> This function wrote 3 registers at one time and so it is not compatible
> with the array-approach.
> 
> I re-added the old function and call this after the default routing table
> was applied. This default routing table can still be used as the only
> connection handled by mmsys on BPI-R2 is OVL0->RDMA0 and this is already
> present in the default routing table
> 
> Fixes: 440147639ac7 ("soc: mediatek: mmsys: Use an array for setting the routing registers")
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>   drivers/soc/mediatek/mtk-mmsys.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index 080660ef11bf..f91b7fdd417a 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -57,6 +57,25 @@ struct mtk_mmsys {
>   	const struct mtk_mmsys_driver_data *data;
>   };
>   
> +static void mtk_mmsys_ddp_sout_sel(struct device *dev,
> +				   enum mtk_ddp_comp_id cur,
> +				   enum mtk_ddp_comp_id next)
> +{
> +	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
> +
> +	if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> +		writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
> +			       mmsys->regs + DISP_REG_CONFIG_OUT_SEL);
> +	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
> +		writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI,
> +			       mmsys->regs + DISP_REG_CONFIG_OUT_SEL);
> +		writel_relaxed(DSI_SEL_IN_RDMA,
> +			       mmsys->regs + DISP_REG_CONFIG_DSI_SEL);
> +		writel_relaxed(DPI_SEL_IN_BLS,
> +			       mmsys->regs + DISP_REG_CONFIG_DPI_SEL);

you can still use the array approach for this like that:

         {
                 DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
                 DISP_REG_CONFIG_OUT_SEL, BLS_TO_DSI_RDMA1_TO_DPI1
         }, {
         {
                 DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
                 DISP_REG_CONFIG_OUT_SEL, BLS_TO_DPI_RDMA1_TO_DSI
         }, {
         {
                 DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
                 DISP_REG_CONFIG_DSI_SEL, DSI_SEL_IN_RDMA
         }, {
         {
                 DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
                 DISP_REG_CONFIG_DPI_SEL, DPI_SEL_IN_BLS
         }, {


Thanks,
Dafna

> +	}
> +}
> +
>   void mtk_mmsys_ddp_connect(struct device *dev,
>   			   enum mtk_ddp_comp_id cur,
>   			   enum mtk_ddp_comp_id next)
> @@ -71,6 +90,8 @@ void mtk_mmsys_ddp_connect(struct device *dev,
>   			reg = readl_relaxed(mmsys->regs + routes[i].addr) | routes[i].val;
>   			writel_relaxed(reg, mmsys->regs + routes[i].addr);
>   		}
> +
> +	mtk_mmsys_ddp_sout_sel(dev, cur, next);
>   }
>   EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect);
>   
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Aw: Re: [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
  2021-07-12 11:50 ` Dafna Hirschfeld
@ 2021-07-12 12:35   ` Frank Wunderlich
  2021-07-12 17:16   ` Frank Wunderlich
  1 sibling, 0 replies; 8+ messages in thread
From: Frank Wunderlich @ 2021-07-12 12:35 UTC (permalink / raw)
  To: Dafna Hirschfeld
  Cc: Frank Wunderlich, linux-mediatek, Matthias Brugger, CK Hu,
	linux-arm-kernel, linux-kernel, Enric Balletbo Serra,
	David Airlie, dri-devel, Daniel Vetter

> Gesendet: Montag, 12. Juli 2021 um 13:50 Uhr
> Von: "Dafna Hirschfeld" <dafna.hirschfeld@collabora.com>
> An: "Frank Wunderlich" <linux@fw-web.de>, linux-mediatek@lists.infradead.org
> Cc: "Frank Wunderlich" <frank-w@public-files.de>, "Matthias Brugger" <matthias.bgg@gmail.com>, "CK Hu" <ck.hu@mediatek.com>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, "Enric Balletbo Serra" <eballetbo@gmail.com>, "David Airlie" <airlied@linux.ie>, dri-devel@lists.freedesktop.org, "Daniel Vetter" <daniel@ffwll.ch>
> Betreff: Re: [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
>
> Hi
>
> On 10.07.21 15:24, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> >
> > HDMI output was broken on mt7623/BPI-R2 in 5.13 because function for
> > special output selection (mtk_mmsys_ddp_sout_sel) was dropped.
> > This function wrote 3 registers at one time and so it is not compatible
> > with the array-approach.
> >
> > I re-added the old function and call this after the default routing table
> > was applied. This default routing table can still be used as the only
> > connection handled by mmsys on BPI-R2 is OVL0->RDMA0 and this is already
> > present in the default routing table
> >
> > Fixes: 440147639ac7 ("soc: mediatek: mmsys: Use an array for setting the routing registers")
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> >   drivers/soc/mediatek/mtk-mmsys.c | 21 +++++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> > index 080660ef11bf..f91b7fdd417a 100644
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > @@ -57,6 +57,25 @@ struct mtk_mmsys {
> >   	const struct mtk_mmsys_driver_data *data;
> >   };
> >
> > +static void mtk_mmsys_ddp_sout_sel(struct device *dev,
> > +				   enum mtk_ddp_comp_id cur,
> > +				   enum mtk_ddp_comp_id next)
> > +{
> > +	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
> > +
> > +	if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> > +		writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
> > +			       mmsys->regs + DISP_REG_CONFIG_OUT_SEL);
> > +	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
> > +		writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI,
> > +			       mmsys->regs + DISP_REG_CONFIG_OUT_SEL);
> > +		writel_relaxed(DSI_SEL_IN_RDMA,
> > +			       mmsys->regs + DISP_REG_CONFIG_DSI_SEL);
> > +		writel_relaxed(DPI_SEL_IN_BLS,
> > +			       mmsys->regs + DISP_REG_CONFIG_DPI_SEL);
>
> you can still use the array approach for this like that:
>
>          {
>                  DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
>                  DISP_REG_CONFIG_OUT_SEL, BLS_TO_DSI_RDMA1_TO_DPI1
>          }, {
>          {
>                  DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
>                  DISP_REG_CONFIG_OUT_SEL, BLS_TO_DPI_RDMA1_TO_DSI
>          }, {
>          {
>                  DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
>                  DISP_REG_CONFIG_DSI_SEL, DSI_SEL_IN_RDMA
>          }, {
>          {
>                  DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
>                  DISP_REG_CONFIG_DPI_SEL, DPI_SEL_IN_BLS
>          }, {
>
>
> Thanks,
> Dafna

these 4 blocks are already there (except your additional brackets) and it does not work with them (not added in 5.14)

 91 static const struct mtk_mmsys_routes mmsys_default_routing_table[] = {
 92     {
 93         DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
 94         DISP_REG_CONFIG_OUT_SEL, BLS_TO_DSI_RDMA1_TO_DPI1 //(1)
 95     }, {
 96         DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
 97         DISP_REG_CONFIG_DSI_SEL, DSI_SEL_IN_BLS
 98     }, {
 99         DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
100         DISP_REG_CONFIG_OUT_SEL, BLS_TO_DPI_RDMA1_TO_DSI //(2)
101     }, {
102         DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
103         DISP_REG_CONFIG_DSI_SEL, DSI_SEL_IN_RDMA //(3)
104     }, {
105         DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
106         DISP_REG_CONFIG_DPI_SEL, DPI_SEL_IN_BLS //(4)
107     }, {

and code should apply all matching parts (2-4 for BLS=>dpi), but it does not work...maybe because new code reads and OR the new value instead of writing it (old code overwrites current register value with the new value without reading)

 69     for (i = 0; i < mmsys->data->num_routes; i++)
 70         if (cur == routes[i].from_comp && next == routes[i].to_comp) {
 71             reg = readl_relaxed(mmsys->regs + routes[i].addr) | routes[i].val; //<<<<<<<<<<<
 72             writel_relaxed(reg, mmsys->regs + routes[i].addr);
 73         }


regards Frank

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Aw: Re: [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
  2021-07-12 11:50 ` Dafna Hirschfeld
  2021-07-12 12:35   ` Aw: " Frank Wunderlich
@ 2021-07-12 17:16   ` Frank Wunderlich
  2021-07-12 17:32     ` Dafna Hirschfeld
  1 sibling, 1 reply; 8+ messages in thread
From: Frank Wunderlich @ 2021-07-12 17:16 UTC (permalink / raw)
  To: Dafna Hirschfeld
  Cc: Frank Wunderlich, linux-mediatek, Matthias Brugger, CK Hu,
	linux-arm-kernel, linux-kernel, Enric Balletbo Serra,
	David Airlie, dri-devel, Daniel Vetter

Hi,

it turns out that problem is the read+or of the new value

i reverted my patch and changed

reg = readl_relaxed(mmsys->regs + routes[i].addr) | routes[i].val;
writel_relaxed(reg, mmsys->regs + routes[i].addr);

to

writel_relaxed(routes[i].val, mmsys->regs + routes[i].addr);

and it works too, but maybe it breaks other platforms

regards Frank

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Aw: Re: [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
  2021-07-12 17:16   ` Frank Wunderlich
@ 2021-07-12 17:32     ` Dafna Hirschfeld
  2021-07-24 17:06       ` Frank Wunderlich
  0 siblings, 1 reply; 8+ messages in thread
From: Dafna Hirschfeld @ 2021-07-12 17:32 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: David Airlie, linux-kernel, dri-devel, linux-mediatek,
	Matthias Brugger, Frank Wunderlich, linux-arm-kernel,
	Enric Balletbo i Serra, Collabora Kernel ML

Hi,

On 12.07.21 19:16, Frank Wunderlich wrote:
> Hi,
> 
> it turns out that problem is the read+or of the new value
> 
> i reverted my patch and changed
> 
> reg = readl_relaxed(mmsys->regs + routes[i].addr) | routes[i].val;
> writel_relaxed(reg, mmsys->regs + routes[i].addr);
> 
> to
> 
> writel_relaxed(routes[i].val, mmsys->regs + routes[i].addr);
> 
> and it works too, but maybe it breaks other platforms

Interesting, I can test if it fix that similar bug on mt8173 when resume from suspend.

Thanks,
Dafna

> 
> regards Frank
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Aw: Re: [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
  2021-07-12 17:32     ` Dafna Hirschfeld
@ 2021-07-24 17:06       ` Frank Wunderlich
  2021-07-26  0:27         ` Chun-Kuang Hu
  0 siblings, 1 reply; 8+ messages in thread
From: Frank Wunderlich @ 2021-07-24 17:06 UTC (permalink / raw)
  To: linux-mediatek, Dafna Hirschfeld, Chun-Kuang Hu
  Cc: David Airlie, linux-kernel, dri-devel, Matthias Brugger,
	Frank Wunderlich, linux-arm-kernel, Enric Balletbo i Serra,
	Collabora Kernel ML

>On 12.07.21 19:16, Frank Wunderlich wrote:
>> Hi,
>> 
>> it turns out that problem is the read+or of the new value
>> 
>> i reverted my patch and changed
>> 
>> reg = readl_relaxed(mmsys->regs + routes[i].addr) | routes[i].val;
>> writel_relaxed(reg, mmsys->regs + routes[i].addr);
>> 
>> to
>> 
>> writel_relaxed(routes[i].val, mmsys->regs + routes[i].addr);
>> 
>> and it works too, but maybe it breaks other platforms

A gentle ping. Amy further comments which of both ways is the right one (restoring old output select function or write only without read+or)?

regards Frank

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Aw: Re: [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
  2021-07-24 17:06       ` Frank Wunderlich
@ 2021-07-26  0:27         ` Chun-Kuang Hu
  2021-07-26 10:33           ` Aw: " Frank Wunderlich
  0 siblings, 1 reply; 8+ messages in thread
From: Chun-Kuang Hu @ 2021-07-26  0:27 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: moderated list:ARM/Mediatek SoC support, Dafna Hirschfeld,
	Chun-Kuang Hu, David Airlie, linux-kernel, DRI Development,
	Enric Balletbo i Serra, Matthias Brugger, Frank Wunderlich,
	Collabora Kernel ML, Linux ARM

Hi, Frank:

Frank Wunderlich <frank-w@public-files.de> 於 2021年7月25日 週日 上午1:06寫道:
>
> >On 12.07.21 19:16, Frank Wunderlich wrote:
> >> Hi,
> >>
> >> it turns out that problem is the read+or of the new value
> >>
> >> i reverted my patch and changed
> >>
> >> reg = readl_relaxed(mmsys->regs + routes[i].addr) | routes[i].val;
> >> writel_relaxed(reg, mmsys->regs + routes[i].addr);
> >>
> >> to
> >>
> >> writel_relaxed(routes[i].val, mmsys->regs + routes[i].addr);
> >>
> >> and it works too, but maybe it breaks other platforms
>
> A gentle ping. Amy further comments which of both ways is the right one (restoring old output select function or write only without read+or)?

As I've discussed in [1], SOUT has many bits and need to be cleared
before set new value. Of course, write only could do the clear, but
for MOUT, it clear the bits that should not be cleared. If you want to
use the table, you need to implement the 'mask'.

[1] https://patchwork.kernel.org/project/linux-mediatek/patch/trinity-937ebfa3-d123-42de-a289-3ad0dbc09782-1625830110576@3c-app-gmx-bap43/

Regards,
Chun-Kuang.

>
> regards Frank

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Aw: Re:  Re: [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2
  2021-07-26  0:27         ` Chun-Kuang Hu
@ 2021-07-26 10:33           ` Frank Wunderlich
  0 siblings, 0 replies; 8+ messages in thread
From: Frank Wunderlich @ 2021-07-26 10:33 UTC (permalink / raw)
  To: Chun-Kuang Hu
  Cc: moderated list:ARM/Mediatek SoC support, Dafna Hirschfeld,
	Chun-Kuang Hu, David Airlie, linux-kernel, DRI Development,
	Enric Balletbo i Serra, Matthias Brugger, Frank Wunderlich,
	Collabora Kernel ML, Linux ARM

Hi

> Gesendet: Montag, 26. Juli 2021 um 02:27 Uhr
> Von: "Chun-Kuang Hu" <chunkuang.hu@kernel.org>
> As I've discussed in [1], SOUT has many bits and need to be cleared
> before set new value. Of course, write only could do the clear, but
> for MOUT, it clear the bits that should not be cleared. If you want to
> use the table, you need to implement the 'mask'.
>
> [1] https://patchwork.kernel.org/project/linux-mediatek/patch/trinity-937ebfa3-d123-42de-a289-3ad0dbc09782-1625830110576@3c-app-gmx-bap43/

I've added the mask like you did here [1] on my repo [2]

and it works too...should i use you as author as i only take your code when posting?

regards Frank

[1] https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2345186/5/drivers/soc/mediatek/mtk-mmsys.c#294
[2] https://github.com/frank-w/BPI-R2-4.14/commits/5.14-mmsys

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-07-26 10:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-10 13:24 [PATCH] soc: mediatek: mmsys: fix HDMI output on mt7623/bananapi-r2 Frank Wunderlich
2021-07-12 11:50 ` Dafna Hirschfeld
2021-07-12 12:35   ` Aw: " Frank Wunderlich
2021-07-12 17:16   ` Frank Wunderlich
2021-07-12 17:32     ` Dafna Hirschfeld
2021-07-24 17:06       ` Frank Wunderlich
2021-07-26  0:27         ` Chun-Kuang Hu
2021-07-26 10:33           ` Aw: " Frank Wunderlich

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).