Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
0 > > > > [1.920576] sun4i_dclk_recalc_rate: val = 1, rate = 17820 > > > > [1.920597] rate = 17820 > > > > [1.920599] parent_rate = 29700 > > > > [1.920602] reg = 0x90c0 > > > > [1.920605] _nkm.n = 3, nkm->n.offset = 0x1, nkm->n.shift = 8 > > > > [1.920609] _nkm.k = 2, nkm->k.offset = 0x1, nkm->k.shift = 4 > > > > [1.920612] _nkm.m = 10, nkm->m.offset = 0x1, nkm->m.shift = 0 > > > > [1.920958] sun4i_dclk_set_rate div 6 > > > > [1.920966] sun4i_dclk_recalc_rate: val = 6, rate = 2970 > > > > > > > > and clk_summary: > > > > > > > > pll-video0111 29700 > > > > 0 0 5 > > > >hdmi 000 29700 > > > > 0 0 5 > > > >tcon1 000 29700 > > > > 0 0 5 > > > >pll-mipi 111 17820 > > > > 0 0 5 > > > > tcon0 221 17820 > > > > 0 0 5 > > > > tcon-pixel-clock 1112970 > > > > 0 0 5 > > > >pll-video0-2x 000 59400 > > > > 0 0 5 > > > > > > This discussion is going nowhere. I'm telling you that your patch > > > doesn't apply the divider you want on the proper clock, and you're > > > replying that indeed, you're applying it on the wrong clock. > > > > > > It might work by accident in your case, but the board I have here > > > clearly indicates otherwise, so there's two possible way out here: > > > > > > - Either you apply that divider to the TCON *module* clock, and not > > > the dclk > > > > > > - Or you point to somewhere in the allwinner code where the bpp / > > > lanes divider is used for the dclk divider. > > > > I don't know how to proceed further on this, as you say it might work > > in accident but I have tested this in A33, A64 and R40 with 4 > > different DSI panels and one DSI-RGB bridge. All of them do use > > PLL_MIPI (pll_rate) and it indeed depends on bpp/lanes > > > > 4-lane, 24-bit: Novatek NT35596 panel > > 4-lane, 24-bit: Feiyang, FY07024di26a30d panel > > 4-lane, 24-bit: Bananapi-s070wv20 panel > > 2-lane, 24-bit: Techstar,ts8550b panel > > > > and > > > > 4-lane, 24-bit, ICN6211 DSI-to-RGB bridge panel > > > > All above listed panels and bridges are working as per BSP and do > > follow bpp/lanes and for DIVIDER 4 no panel is working. > > Look. I'm not saying that there's no issue, I'm saying that your > patch, applied to the clock you're applying it to, doesn't make sense > and isn't what the BSP does. tcon-pixel clock is the rate that you want to achive on display side and if you have 4 lanes 32bit or lanes and different bit number that you need to have a clock that is able to put outside bits and speed equal to pixel-clock * bits / lanes. so If you want a pixel-clock of 40 mhz and you have 32bits and 4 lanes you need to have a clock of 40 * 32 / 4 in no-burst mode. I think that this is done but most of the display. Now in burst mode I don't know how should work the calculation of the clock for the require bandwidth and even I understand your comment I would like to have your clock tree after you boot on the display side and if it is possible I want to assemble a kit like you have. > > You can keep on arguing that your patch is perfect as is, but the fact > that there's regressions proves otherwise. > Well when you push your code you said that you have tested on more then one display. Can I know where are the others? > > The panels/bridges I have has tested in BSP and as you mentioned in > > another mail, your panel is not tested in BSP - this is the only > > difference. I did much reverse-engineering on PLL_MIPI clocking in BSP > > so I'm afraid what can I do next on this, If you want to look further > > on BSP I would suggest to verify on pll_rate side. If you feel > > anything I'm missing please let me know. > > I already told you how we can make some progress in the mail you > quoted, but you chose to ignore that. > Yes, the idea is to make progress. Thank you about your helping Michael > Until there's been some progress on either points mentionned above, > I'm just going to stop answering on this topic. > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com -- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com |
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Hi On Thu, Jul 11, 2019 at 7:43 PM Michael Nazzareno Trimarchi wrote: > > Hi Maxime > > On Thu, Jul 11, 2019 at 2:23 PM Maxime Ripard > wrote: > > > > On Fri, Jul 05, 2019 at 07:52:27PM +0200, Michael Nazzareno Trimarchi wrote: > > > On Wed, Jul 3, 2019 at 1:49 PM Maxime Ripard > > > wrote: > > > > > > > > On Tue, Jun 25, 2019 at 09:00:36PM +0530, Jagan Teki wrote: > > > > > On Tue, Jun 25, 2019 at 8:19 PM Maxime Ripard > > > > > wrote: > > > > > > > > > > > > On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote: > > > > > > > On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard > > > > > > > wrote: > > > > > > > > > > > > > > > > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote: > > > > > > > > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > I've reordered the mail a bit to work on chunks > > > > > > > > > > > > > > > > > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote: > > > > > > > > > > > > I wish it was in your commit log in the first place, > > > > > > > > > > > > instead of having > > > > > > > > > > > > to exchange multiple mails over this. > > > > > > > > > > > > > > > > > > > > > > > > However, I don't think that's quite true, and it might > > > > > > > > > > > > be a bug in > > > > > > > > > > > > Allwinner's implementation (or rather something quite > > > > > > > > > > > > confusing). > > > > > > > > > > > > > > > > > > > > > > > > You're right that the lcd_rate and pll_rate seem to be > > > > > > > > > > > > generated from > > > > > > > > > > > > the pixel clock, and it indeed looks like the ratio > > > > > > > > > > > > between the pixel > > > > > > > > > > > > clock and the TCON dotclock is defined through the > > > > > > > > > > > > number of bits per > > > > > > > > > > > > lanes. > > > > > > > > > > > > > > > > > > > > > > > > However, in this case, dsi_rate is actually the same > > > > > > > > > > > > than lcd_rate, > > > > > > > > > > > > since pll_rate is going to be divided by dsi_div: > > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L791 > > > > > > > > > > > > > > > > > > > > > > > > Since lcd_div is 1, it also means that in this case, > > > > > > > > > > > > dsi_rate == > > > > > > > > > > > > dclk_rate. > > > > > > > > > > > > > > > > > > > > > > > > The DSI module clock however, is always set to 148.5 > > > > > > > > > > > > MHz. Indeed, if > > > > > > > > > > > > we look at: > > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L804 > > > > > > > > > > > > > > > > > > > > > > > > We can see that the rate in clk_info is used if it's > > > > > > > > > > > > different than > > > > > > > > > > > > 0. This is filled by disp_al_lcd_get_clk_info, which, > > > > > > > > > > > > in the case of a > > > > > > > > > > > > DSI panel, will hardcode it to 148.5 MHz: > > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Hi Maxime On Thu, Jul 11, 2019 at 2:23 PM Maxime Ripard wrote: > > On Fri, Jul 05, 2019 at 07:52:27PM +0200, Michael Nazzareno Trimarchi wrote: > > On Wed, Jul 3, 2019 at 1:49 PM Maxime Ripard > > wrote: > > > > > > On Tue, Jun 25, 2019 at 09:00:36PM +0530, Jagan Teki wrote: > > > > On Tue, Jun 25, 2019 at 8:19 PM Maxime Ripard > > > > wrote: > > > > > > > > > > On Thu, Jun 20, 2019 at 11:57:44PM +0530, Jagan Teki wrote: > > > > > > On Fri, Jun 14, 2019 at 7:54 PM Maxime Ripard > > > > > > wrote: > > > > > > > > > > > > > > On Wed, Jun 05, 2019 at 01:03:16PM +0530, Jagan Teki wrote: > > > > > > > > On Wed, Jun 5, 2019 at 12:19 PM Maxime Ripard > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > I've reordered the mail a bit to work on chunks > > > > > > > > > > > > > > > > > > On Fri, May 24, 2019 at 03:37:42PM +0530, Jagan Teki wrote: > > > > > > > > > > > I wish it was in your commit log in the first place, > > > > > > > > > > > instead of having > > > > > > > > > > > to exchange multiple mails over this. > > > > > > > > > > > > > > > > > > > > > > However, I don't think that's quite true, and it might be > > > > > > > > > > > a bug in > > > > > > > > > > > Allwinner's implementation (or rather something quite > > > > > > > > > > > confusing). > > > > > > > > > > > > > > > > > > > > > > You're right that the lcd_rate and pll_rate seem to be > > > > > > > > > > > generated from > > > > > > > > > > > the pixel clock, and it indeed looks like the ratio > > > > > > > > > > > between the pixel > > > > > > > > > > > clock and the TCON dotclock is defined through the number > > > > > > > > > > > of bits per > > > > > > > > > > > lanes. > > > > > > > > > > > > > > > > > > > > > > However, in this case, dsi_rate is actually the same than > > > > > > > > > > > lcd_rate, > > > > > > > > > > > since pll_rate is going to be divided by dsi_div: > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L791 > > > > > > > > > > > > > > > > > > > > > > Since lcd_div is 1, it also means that in this case, > > > > > > > > > > > dsi_rate == > > > > > > > > > > > dclk_rate. > > > > > > > > > > > > > > > > > > > > > > The DSI module clock however, is always set to 148.5 MHz. > > > > > > > > > > > Indeed, if > > > > > > > > > > > we look at: > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/disp_lcd.c#L804 > > > > > > > > > > > > > > > > > > > > > > We can see that the rate in clk_info is used if it's > > > > > > > > > > > different than > > > > > > > > > > > 0. This is filled by disp_al_lcd_get_clk_info, which, in > > > > > > > > > > > the case of a > > > > > > > > > > > DSI panel, will hardcode it to 148.5 MHz: > > > > > > > > > > > https://github.com/BPI-SINOVOIP/BPI-M64-bsp/blob/master/linux-sunxi/drivers/video/sunxi/disp2/disp/de/lowlevel_sun50iw1/disp_al.c#L164 > > > > > > > > > > > > > > > > > > > > Let me explain, something more. > > > > > > > > > > > > > > > > > > > > According to bsp there are clk_info.tcon_div which I will > > > > > > > > > > explain below. > > >
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Hi Maxime
On Tue, Aug 13, 2019 at 8:05 AM Maxime Ripard wrote:
>
> On Mon, Jul 29, 2019 at 08:59:04AM +0200, Michael Nazzareno Trimarchi wrote:
> > Hi
> >
> > On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
> > wrote:
> > >
> > > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > > Hi Maxime,
> > > >
> > > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard
> > > > wrote:
> > > > >
> > > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > > > wrote:
> > > > > > >
> > > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno
> > > > > > > Trimarchi wrote:
> > > > > > > > > > tcon-pixel clock is the rate that you want to achive on
> > > > > > > > > > display side
> > > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit
> > > > > > > > > > number that
> > > > > > > > > > you need to have a clock that is able to put outside bits
> > > > > > > > > > and speed
> > > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a
> > > > > > > > > > pixel-clock of
> > > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a
> > > > > > > > > > clock of
> > > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but
> > > > > > > > > > most of
> > > > > > > > > > the display.
> > > > > > > > >
> > > > > > > > > So this is what the issue is then?
> > > > > > > > >
> > > > > > > > > This one does make sense, and you should just change the rate
> > > > > > > > > in the
> > > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > > >
> > > > > > > > > I'm still wondering why that hasn't been brought up in either
> > > > > > > > > the
> > > > > > > > > discussion or the commit log before though.
> > > > > > > > >
> > > > > > > > Something like this?
> > > > > > > >
> > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 --
> > > > > > > > 2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const
> > > > > > > > struct
> > > > > > > > drm_display_mode *mode,
> > > > > > > > }
> > > > > > > >
> > > > > > > > static void sun4i_tcon0_mode_set_common(struct sun4i_tcon
> > > > > > > > *tcon,
> > > > > > > > - const struct
> > > > > > > > drm_display_mode *mode)
> > > > > > > > + const struct
> > > > > > > > drm_display_mode *mode,
> > > > > > > > + u32 tcon_mul)
> > > > > > > > {
> > > > > > > > /* Configure the dot clock */
> > > > > > > > - clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > > + clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul *
> > > > > > > > 1000);
> > > > > > > >
> > > > > > > > /* Set the resolution */
> > > > > > > > regmap_write(tcon->regs, SUN
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Hi Maxime On Wed, Aug 28, 2019 at 3:03 PM Maxime Ripard wrote: > > Hi, > > On Thu, Aug 15, 2019 at 02:25:57PM +0200, Michael Nazzareno Trimarchi wrote: > > On Tue, Aug 13, 2019 at 8:05 AM Maxime Ripard > > wrote: > > > On Mon, Jul 29, 2019 at 08:59:04AM +0200, Michael Nazzareno Trimarchi > > > wrote: > > > > Hi > > > > > > > > On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard > > > > wrote: > > > > > > > > > > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote: > > > > > > Hi Maxime, > > > > > > > > > > > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard > > > > > > wrote: > > > > > > > > > > > > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote: > > > > > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno > > > > > > > > > Trimarchi wrote: > > > > > > > > > > > > tcon-pixel clock is the rate that you want to achive on > > > > > > > > > > > > display side > > > > > > > > > > > > and if you have 4 lanes 32bit or lanes and different > > > > > > > > > > > > bit number that > > > > > > > > > > > > you need to have a clock that is able to put outside > > > > > > > > > > > > bits and speed > > > > > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a > > > > > > > > > > > > pixel-clock of > > > > > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have > > > > > > > > > > > > a clock of > > > > > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done > > > > > > > > > > > > but most of > > > > > > > > > > > > the display. > > > > > > > > > > > > > > > > > > > > > > So this is what the issue is then? > > > > > > > > > > > > > > > > > > > > > > This one does make sense, and you should just change the > > > > > > > > > > > rate in the > > > > > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu. > > > > > > > > > > > > > > > > > > > > > > I'm still wondering why that hasn't been brought up in > > > > > > > > > > > either the > > > > > > > > > > > discussion or the commit log before though. > > > > > > > > > > > > > > > > > > > > > Something like this? > > > > > > > > > > > > > > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 > > > > > > > > > > +++- > > > > > > > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 -- > > > > > > > > > > 2 files changed, 11 insertions(+), 11 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > > > > > > index 64c43ee6bd92..42560d5c327c 100644 > > > > > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > > > > > > @@ -263,10 +263,11 @@ static int > > > > > > > > > > sun4i_tcon_get_clk_delay(const struct > > > > > > > > > > drm_display_mode *mode, > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > static void sun4i_tcon0_mode_set_common(struct sun4i_tcon > > > > > > > > > > *tcon, > > > > > > > > > > - const struct > > > > > > > >
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Hi Maxime
On Mon, Jul 29, 2019 at 8:59 AM Michael Nazzareno Trimarchi
wrote:
>
> Hi
>
> On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
> wrote:
> >
> > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > Hi Maxime,
> > >
> > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard
> > > wrote:
> > > >
> > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > > wrote:
> > > > > >
> > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno
> > > > > > Trimarchi wrote:
> > > > > > > > > tcon-pixel clock is the rate that you want to achive on
> > > > > > > > > display side
> > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit
> > > > > > > > > number that
> > > > > > > > > you need to have a clock that is able to put outside bits and
> > > > > > > > > speed
> > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a
> > > > > > > > > pixel-clock of
> > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a
> > > > > > > > > clock of
> > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but
> > > > > > > > > most of
> > > > > > > > > the display.
> > > > > > > >
> > > > > > > > So this is what the issue is then?
> > > > > > > >
> > > > > > > > This one does make sense, and you should just change the rate
> > > > > > > > in the
> > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > >
> > > > > > > > I'm still wondering why that hasn't been brought up in either
> > > > > > > > the
> > > > > > > > discussion or the commit log before though.
> > > > > > > >
> > > > > > > Something like this?
> > > > > > >
> > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 --
> > > > > > > 2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const
> > > > > > > struct
> > > > > > > drm_display_mode *mode,
> > > > > > > }
> > > > > > >
> > > > > > > static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > > - const struct
> > > > > > > drm_display_mode *mode)
> > > > > > > + const struct
> > > > > > > drm_display_mode *mode,
> > > > > > > + u32 tcon_mul)
> > > > > > > {
> > > > > > > /* Configure the dot clock */
> > > > > > > - clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > + clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul *
> > > > > > > 1000);
> > > > > > >
> > > > > > > /* Set the resolution */
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > u8 lanes = device->lanes;
> > > > > > > u32 block_space, start_delay;
> > > > > > > -
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Hi
On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
wrote:
>
> On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > Hi Maxime,
> >
> > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard
> > wrote:
> > >
> > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > wrote:
> > > > >
> > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi
> > > > > wrote:
> > > > > > > > tcon-pixel clock is the rate that you want to achive on display
> > > > > > > > side
> > > > > > > > and if you have 4 lanes 32bit or lanes and different bit number
> > > > > > > > that
> > > > > > > > you need to have a clock that is able to put outside bits and
> > > > > > > > speed
> > > > > > > > equal to pixel-clock * bits / lanes. so If you want a
> > > > > > > > pixel-clock of
> > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock
> > > > > > > > of
> > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but
> > > > > > > > most of
> > > > > > > > the display.
> > > > > > >
> > > > > > > So this is what the issue is then?
> > > > > > >
> > > > > > > This one does make sense, and you should just change the rate in
> > > > > > > the
> > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > >
> > > > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > > > discussion or the commit log before though.
> > > > > > >
> > > > > > Something like this?
> > > > > >
> > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 --
> > > > > > 2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const
> > > > > > struct
> > > > > > drm_display_mode *mode,
> > > > > > }
> > > > > >
> > > > > > static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > - const struct
> > > > > > drm_display_mode *mode)
> > > > > > + const struct
> > > > > > drm_display_mode *mode,
> > > > > > + u32 tcon_mul)
> > > > > > {
> > > > > > /* Configure the dot clock */
> > > > > > - clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > + clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul *
> > > > > > 1000);
> > > > > >
> > > > > > /* Set the resolution */
> > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > sun4i_tcon *tcon,
> > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > u8 lanes = device->lanes;
> > > > > > u32 block_space, start_delay;
> > > > > > - u32 tcon_div;
> > > > > > + u32 tcon_div, tcon_mul;
> > > > > >
> > > > > > - tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > - tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > + tcon->dclk_min_div = 4;
> > > > > > + tcon->dclk_max_div = 127;
> > > > > >
> > > > > > -
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Hi
On Sat, Jul 20, 2019 at 11:32 AM Maxime Ripard
wrote:
>
> On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > wrote:
> > >
> > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi
> > > wrote:
> > > > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > > > you need to have a clock that is able to put outside bits and speed
> > > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > > > the display.
> > > > >
> > > > > So this is what the issue is then?
> > > > >
> > > > > This one does make sense, and you should just change the rate in the
> > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > >
> > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > discussion or the commit log before though.
> > > > >
> > > > Something like this?
> > > >
> > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 --
> > > > 2 files changed, 11 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > > drm_display_mode *mode,
> > > > }
> > > >
> > > > static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > - const struct drm_display_mode
> > > > *mode)
> > > > + const struct drm_display_mode
> > > > *mode,
> > > > + u32 tcon_mul)
> > > > {
> > > > /* Configure the dot clock */
> > > > - clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > + clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > > >
> > > > /* Set the resolution */
> > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > sun4i_tcon *tcon,
> > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > u8 lanes = device->lanes;
> > > > u32 block_space, start_delay;
> > > > - u32 tcon_div;
> > > > + u32 tcon_div, tcon_mul;
> > > >
> > > > - tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > - tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > + tcon->dclk_min_div = 4;
> > > > + tcon->dclk_max_div = 127;
> > > >
> > > > - sun4i_tcon0_mode_set_common(tcon, mode);
> > > > + tcon_mul = bpp / lanes;
> > > > + sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > >
> > > > /* Set dithering if needed */
> > > > sun4i_tcon0_mode_set_dithering(tcon,
> > > > sun4i_tcon_get_connector(encoder));
> > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > sun4i_tcon *tcon,
> > > > */
> > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > > > tcon_div &= GENMASK(6, 0);
> > > > - block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > + block_space = mode->htotal * tcon_div * tcon_mul;
> > > > block_space -= mode->hdisplay + 40;
> > > >
> > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > sun4i_tcon *tcon,
> > > >
> > > > tcon->dclk_min_div = 7
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Hi
On Sat., 20 Jul. 2019, 8:58 am Maxime Ripard,
wrote:
> On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi
> wrote:
> > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > you need to have a clock that is able to put outside bits and speed
> > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > the display.
> > >
> > > So this is what the issue is then?
> > >
> > > This one does make sense, and you should just change the rate in the
> > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > >
> > > I'm still wondering why that hasn't been brought up in either the
> > > discussion or the commit log before though.
> > >
> > Something like this?
> >
> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 --
> > 2 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > index 64c43ee6bd92..42560d5c327c 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > drm_display_mode *mode,
> > }
> >
> > static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > - const struct drm_display_mode
> *mode)
> > + const struct drm_display_mode
> *mode,
> > + u32 tcon_mul)
> > {
> > /* Configure the dot clock */
> > - clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > + clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> >
> > /* Set the resolution */
> > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > sun4i_tcon *tcon,
> > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > u8 lanes = device->lanes;
> > u32 block_space, start_delay;
> > - u32 tcon_div;
> > + u32 tcon_div, tcon_mul;
> >
> > - tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > - tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > + tcon->dclk_min_div = 4;
> > + tcon->dclk_max_div = 127;
> >
> > - sun4i_tcon0_mode_set_common(tcon, mode);
> > + tcon_mul = bpp / lanes;
> > + sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> >
> > /* Set dithering if needed */
> > sun4i_tcon0_mode_set_dithering(tcon,
> sun4i_tcon_get_connector(encoder));
> > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > sun4i_tcon *tcon,
> > */
> > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, _div);
> > tcon_div &= GENMASK(6, 0);
> > - block_space = mode->htotal * bpp / (tcon_div * lanes);
> > + block_space = mode->htotal * tcon_div * tcon_mul;
> > block_space -= mode->hdisplay + 40;
> >
> > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > sun4i_tcon *tcon,
> >
> > tcon->dclk_min_div = 7;
> > tcon->dclk_max_div = 7;
> > - sun4i_tcon0_mode_set_common(tcon, mode);
> > + sun4i_tcon0_mode_set_common(tcon, mode, 1);
> >
> > /* Set dithering if needed */
> > sun4i_tcon0_mode_set_dithering(tcon,
> sun4i_tcon_get_connector(encoder));
> > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > sun4i_tcon *tcon,
> >
> > tcon->dclk_min_div = 6;
> > tcon->dclk_max_div = 127;
> > - sun4i_tcon0_mode_set_common(tcon, mode);
> > + sun4i_tcon0_mode_set_common(tcon, mode, 1);
> >
> > /* Set dithering if needed */
> > sun4i_tcon0_mode_set_dithering(tcon, connector);
> > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > index 5c3ad5be0690..a07090579f84 100644
> > --- a/drivers/gpu/drm/sun4i/sun6i_
Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
Hi Jagan
On Mon, Jul 22, 2019 at 12:21 PM Jagan Teki wrote:
>
> Hi Maxime,
>
> On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard
> wrote:
> >
> > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > wrote:
> > > >
> > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi
> > > > wrote:
> > > > > > > tcon-pixel clock is the rate that you want to achive on display
> > > > > > > side
> > > > > > > and if you have 4 lanes 32bit or lanes and different bit number
> > > > > > > that
> > > > > > > you need to have a clock that is able to put outside bits and
> > > > > > > speed
> > > > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock
> > > > > > > of
> > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most
> > > > > > > of
> > > > > > > the display.
> > > > > >
> > > > > > So this is what the issue is then?
> > > > > >
> > > > > > This one does make sense, and you should just change the rate in the
> > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > >
> > > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > > discussion or the commit log before though.
> > > > > >
> > > > > Something like this?
> > > > >
> > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 --
> > > > > 2 files changed, 11 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > > > drm_display_mode *mode,
> > > > > }
> > > > >
> > > > > static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > - const struct drm_display_mode
> > > > > *mode)
> > > > > + const struct drm_display_mode
> > > > > *mode,
> > > > > + u32 tcon_mul)
> > > > > {
> > > > > /* Configure the dot clock */
> > > > > - clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > + clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > > > >
> > > > > /* Set the resolution */
> > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > sun4i_tcon *tcon,
> > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > u8 lanes = device->lanes;
> > > > > u32 block_space, start_delay;
> > > > > - u32 tcon_div;
> > > > > + u32 tcon_div, tcon_mul;
> > > > >
> > > > > - tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > - tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > + tcon->dclk_min_div = 4;
> > > > > + tcon->dclk_max_div = 127;
> > > > >
> > > > > - sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > + tcon_mul = bpp / lanes;
> > > > > + sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > >
> > > > > /* Set dithering if needed */
> > > > > sun4i_tcon0_mode_set_dithering(tcon,
> > > > > sun4i_tcon_get_connector(encoder));
> > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > sun4i_tcon *tcon,
> > > > > */
> > > > >
Re: [PATCH 2/2] drm: bridge: Add SN65DSI84 DSI to LVDS bridge
On Wed, Jan 20, 2021 at 12:29 PM Jagan Teki wrote: > > On Wed, Jan 20, 2021 at 4:55 PM Michael Nazzareno Trimarchi > wrote: > > > > Hi Jagan > > > > On Wed, Jan 20, 2021 at 12:22 PM Jagan Teki > > wrote: > > > > > > SN65DSI84 is a Single Channel DSI to Dual-link LVDS bridge from > > > Texas Instruments. > > > > > > SN65DSI83, SN65DSI85 are variants of the same family of bridge > > > controllers. > > > > > > Right now the bridge driver is supporting a single link, dual-link > > > support requires to initiate I2C Channel B registers. > > > > > > Tested with STM32MP1 MIPI DSI host design configuration. > > > > > > Signed-off-by: Matteo Lisi > > > Signed-off-by: Jagan Teki > > > --- > > > MAINTAINERS | 6 + > > > drivers/gpu/drm/bridge/Kconfig| 19 + > > > drivers/gpu/drm/bridge/Makefile | 1 + > > > drivers/gpu/drm/bridge/ti-sn65dsi84.c | 488 ++ > > > 4 files changed, 514 insertions(+) > > > create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi84.c > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 12dd1fff2a39..44750ff7640c 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -5984,6 +5984,12 @@ S: Maintained > > > F: Documentation/devicetree/bindings/display/ti/ > > > F: drivers/gpu/drm/omapdrm/ > > > > > > +DRM DRIVERS FOR TI SN65DSI84 DSI TO LVDS BRIDGE > > > +M: Jagan Teki > > > +S: Maintained > > > +F: Documentation/devicetree/bindings/display/bridge/ti,sn65dsi84.yaml > > > +F: drivers/gpu/drm/bridge/ti-sn65dsi84.c > > > + > > > DRM DRIVERS FOR V3D > > > M: Eric Anholt > > > S: Supported > > > diff --git a/drivers/gpu/drm/bridge/Kconfig > > > b/drivers/gpu/drm/bridge/Kconfig > > > index e4110d6ca7b3..6494881bffb3 100644 > > > --- a/drivers/gpu/drm/bridge/Kconfig > > > +++ b/drivers/gpu/drm/bridge/Kconfig > > > @@ -232,6 +232,25 @@ config DRM_TI_TFP410 > > > help > > > Texas Instruments TFP410 DVI/HDMI Transmitter driver > > > > > > +config DRM_TI_SN65DSI84 > > > + tristate "TI SN65DSI84 DSI to LVDS bridge" > > > + depends on OF > > > + select DRM_KMS_HELPER > > > + select REGMAP_I2C > > > + select DRM_PANEL > > > + select DRM_MIPI_DSI > > > + help > > > + Texas Instruments SN65DSI84 Single Channel DSI to Dual-link LVDS > > > + bridge driver. > > > + > > > + Bridge decodes MIPI DSI 18bpp RGB666 and 240bpp RG888 packets > > > and > > > + converts the formatted video data stream to a FlatLink > > > compatible > > > + LVDS output operating at pixel clocks operating from 25 MHx to > > > + 154 MHz. > > > + > > > + SN65DSI84 offers a Dual-Link LVDS, Single-Link LVDS interface > > > with > > > + four data lanes per link. > > > + > > > config DRM_TI_SN65DSI86 > > > tristate "TI SN65DSI86 DSI to eDP bridge" > > > depends on OF > > > diff --git a/drivers/gpu/drm/bridge/Makefile > > > b/drivers/gpu/drm/bridge/Makefile > > > index 86e7acc76f8d..3906052ef639 100644 > > > --- a/drivers/gpu/drm/bridge/Makefile > > > +++ b/drivers/gpu/drm/bridge/Makefile > > > @@ -20,6 +20,7 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o > > > obj-$(CONFIG_DRM_TOSHIBA_TC358768) += tc358768.o > > > obj-$(CONFIG_DRM_TOSHIBA_TC358775) += tc358775.o > > > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ > > > +obj-$(CONFIG_DRM_TI_SN65DSI84) += ti-sn65dsi84.o > > > obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o > > > obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o > > > obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi84.c > > > b/drivers/gpu/drm/bridge/ti-sn65dsi84.c > > > new file mode 100644 > > > index ..3ed1f9a7d898 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi84.c > > > @@ -0,0 +1,488 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright (c) 2021 Engicam srl > > > + * Copyright (C) 2021 Amarula Solutions(India) >
Re: [PATCH 2/2] drm: bridge: Add SN65DSI84 DSI to LVDS bridge
5dsi *sn = bridge_to_sn65dsi(bridge);
> +
> + gpiod_set_value_cansleep(sn->enable, 0);
> +
> + msleep(10);
> +
> + gpiod_set_value_cansleep(sn->enable, 1);
> +
> + msleep(10);
> +}
> +
> +static int sn65dsi_attach(struct drm_bridge *bridge, enum
> drm_bridge_attach_flags flags)
> +{
> + struct sn65dsi *sn = bridge_to_sn65dsi(bridge);
> + struct mipi_dsi_host *host;
> + struct mipi_dsi_device *dsi;
> + const struct mipi_dsi_device_info info = { .type = "sn65dsi",
> + .channel = 0,
> + .node = NULL,
> +};
> + int ret;
> +
> + host = of_find_mipi_dsi_host_by_node(sn->host_node);
> + if (!host) {
> + DRM_DEV_ERROR(sn->dev, "failed to find dsi host\n");
> + return -EPROBE_DEFER;
> + }
> +
> + dsi = mipi_dsi_device_register_full(host, );
> + if (IS_ERR(dsi)) {
> + DRM_DEV_ERROR(sn->dev, "failed to create dsi device\n");
> + return PTR_ERR(sn->dsi);
> + }
> +
> + sn->dsi = dsi;
> + dsi->lanes = sn->dsi_lanes;
> + dsi->format = MIPI_DSI_FMT_RGB888;
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
> +
> + ret = mipi_dsi_attach(dsi);
> + if (ret) {
> + DRM_DEV_ERROR(sn->dev, "failed to attach dsi host\n");
> + goto err_dsi_attach;
> + }
> +
> + return drm_bridge_attach(bridge->encoder, sn->panel_bridge,
> +>bridge, flags);
> +
> +err_dsi_attach:
> + mipi_dsi_device_unregister(dsi);
> + return ret;
> +}
> +
> +static const struct drm_bridge_funcs sn65dsi_bridge_funcs = {
> + .attach = sn65dsi_attach,
> + .pre_enable = sn65dsi_pre_enable,
> + .enable = sn65dsi_enable,
> + .disable= sn65dsi_disable,
> + .post_disable = sn65dsi_post_disable,
> +};
> +
> +static int sn65dsi_parse_dt(struct sn65dsi *sn)
> +{
> + struct device *dev = sn->dev;
> + struct device_node *endpoint, *parent;
> + struct property *prop;
> + struct drm_panel *panel;
> + int len = 0;
> + int ret;
> +
> + sn->enable = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
> + if (IS_ERR(sn->enable)) {
> + DRM_DEV_ERROR(dev, "failed to get enable gpio\n");
> + return PTR_ERR(sn->enable);
> + }
> +
> + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, , NULL);
> + if (ret < 0)
> + return ret;
> + if (!panel)
> + return -ENODEV;
> +
> + sn->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> + if (IS_ERR(sn->panel_bridge))
> + return PTR_ERR(sn->panel_bridge);
> +
> + /*
> +* To get the data-lanes of dsi, we need to access the port1 of
> dsi_out
> +* from the port0 of bridge.
> +*/
> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
> + if (endpoint) {
> + /* dsi_out node */
> + parent = of_graph_get_remote_port_parent(endpoint);
> + of_node_put(endpoint);
> + if (parent) {
> + /* dsi port 1 */
> + endpoint = of_graph_get_endpoint_by_regs(parent, 1,
> -1);
> + of_node_put(parent);
> + if (endpoint) {
> + prop = of_find_property(endpoint,
> "data-lanes", );
> + of_node_put(endpoint);
> + if (!prop) {
> + DRM_DEV_ERROR(dev, "failed to find
> data lane\n");
> + return -EPROBE_DEFER;
> + }
> + }
> + }
> + }
> +
> + sn->dsi_lanes = len / sizeof(u32);
> + if (sn->dsi_lanes < 1 || sn->dsi_lanes > 4)
> + return -EINVAL;
> +
> + sn->host_node = of_graph_get_remote_node(dev->of_node, 0, 0);
> + if (!sn->host_node)
> + return -ENODEV;
> +
> + of_node_put(sn->host_node);
> +
> + return 0;
> +}
> +
> +static int sn65dsi_probe(struct i2c_client *client)
> +{
> + struct sn65dsi *sn;
> + int ret;
&
Re: [PATCH v2 2/2] drm: bridge: Add SN65DSI84 DSI to LVDS bridge
ble, 1);
> +
> + msleep(10);
Ditto
> +}
> +
> +static int sn65dsi_attach(struct drm_bridge *bridge, enum
> drm_bridge_attach_flags flags)
> +{
> + struct sn65dsi *sn = bridge_to_sn65dsi(bridge);
> + struct mipi_dsi_host *host;
> + struct mipi_dsi_device *dsi;
> + const struct mipi_dsi_device_info info = { .type = "sn65dsi",
> + .channel = 0,
> + .node = NULL,
> +};
> + int ret;
> +
> + host = of_find_mipi_dsi_host_by_node(sn->host_node);
> + if (!host) {
> + DRM_DEV_ERROR(sn->dev, "failed to find dsi host\n");
> + return -EPROBE_DEFER;
> + }
> +
> + dsi = mipi_dsi_device_register_full(host, );
> + if (IS_ERR(dsi)) {
> + DRM_DEV_ERROR(sn->dev, "failed to create dsi device\n");
> + return PTR_ERR(sn->dsi);
> + }
> +
> + sn->dsi = dsi;
> + dsi->lanes = sn->dsi_lanes;
> + dsi->format = MIPI_DSI_FMT_RGB888;
> + dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
> +
> + ret = mipi_dsi_attach(dsi);
> + if (ret) {
> + DRM_DEV_ERROR(sn->dev, "failed to attach dsi host\n");
> + goto err_dsi_attach;
> + }
> +
> + return drm_bridge_attach(bridge->encoder, sn->panel_bridge,
> +>bridge, flags);
> +
> +err_dsi_attach:
> + mipi_dsi_device_unregister(dsi);
> + return ret;
> +}
> +
> +static const struct drm_bridge_funcs sn65dsi_bridge_funcs = {
> + .attach = sn65dsi_attach,
> + .pre_enable = sn65dsi_pre_enable,
> + .enable = sn65dsi_enable,
> + .disable= sn65dsi_disable,
> + .post_disable = sn65dsi_post_disable,
> +};
> +
> +static int sn65dsi_parse_dt(struct sn65dsi *sn)
> +{
> + struct device *dev = sn->dev;
> + struct device_node *endpoint, *parent;
> + struct property *prop;
> + struct drm_panel *panel;
> + int len = 0;
> + int ret;
> +
> + sn->enable = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
> + if (IS_ERR(sn->enable)) {
> + DRM_DEV_ERROR(dev, "failed to get enable gpio\n");
> + return PTR_ERR(sn->enable);
> + }
> +
> + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, , NULL);
> + if (ret < 0)
> + return ret;
> + if (!panel)
> + return -ENODEV;
> +
> + sn->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> + if (IS_ERR(sn->panel_bridge))
> + return PTR_ERR(sn->panel_bridge);
> +
> + /*
> +* To get the data-lanes of dsi, we need to access the port1 of
> dsi_out
> +* from the port0 of bridge.
> +*/
> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
> + if (endpoint) {
What is happen if you have no endpoint should not already fail?
> + /* dsi_out node */
> + parent = of_graph_get_remote_port_parent(endpoint);
> + of_node_put(endpoint);
> + if (parent) {
> + /* dsi port 1 */
> + endpoint = of_graph_get_endpoint_by_regs(parent, 1,
> -1);
> + of_node_put(parent);
> + if (endpoint) {
> + prop = of_find_property(endpoint,
> "data-lanes", );
> + of_node_put(endpoint);
> + if (!prop) {
> + DRM_DEV_ERROR(dev, "failed to find
> data lane\n");
> + return -EPROBE_DEFER;
> + }
> + }
> + }
> + }
> +
> + sn->dsi_lanes = len / sizeof(u32);
> + if (sn->dsi_lanes < 1 || sn->dsi_lanes > 4)
> + return -EINVAL;
> +
> + sn->host_node = of_graph_get_remote_node(dev->of_node, 0, 0);
> + if (!sn->host_node)
> + return -ENODEV;
> +
> + of_node_put(sn->host_node);
> +
> + return 0;
> +}
> +
> +static int sn65dsi_probe(struct i2c_client *client)
> +{
> + struct sn65dsi *sn;
> + int ret;
> +
> + sn = devm_kzalloc(>dev, sizeof(*sn), GFP_KERNEL);
> + if (!sn)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, sn);
> + sn->dev = >dev;
> +
> + sn->regmap = devm_regmap_init_i2c(client, _regmap_config);
> + if (IS_ERR(sn->regmap)) {
> + DRM_DEV_ERROR(>dev,
> + "regmap allocation failed (ret = %d)\n", ret);
> + return PTR_ERR(sn->regmap);
> + }
> +
> + ret = sn65dsi_parse_dt(sn);
> + if (ret)
> + return ret;
> +
> + sn->bridge.funcs = _bridge_funcs;
> + sn->bridge.of_node = client->dev.of_node;
> +
> + drm_bridge_add(>bridge);
> +
> + return 0;
> +}
> +
> +static int sn65dsi_remove(struct i2c_client *client)
> +{
> + struct sn65dsi *sn = i2c_get_clientdata(client);
> +
> + drm_bridge_remove(>bridge);
> +
> + return 0;
> +}
> +
> +static struct i2c_device_id sn65dsi_i2c_id[] = {
> + { "sn65dsi84", 0},
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, sn65dsi_i2c_id);
> +
> +static const struct of_device_id sn65dsi_match_table[] = {
> + {.compatible = "ti,sn65dsi84"},
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sn65dsi_match_table);
> +
> +static struct i2c_driver sn65dsi_driver = {
> + .driver = {
> + .name = "ti-sn65dsi84",
> + .of_match_table = sn65dsi_match_table,
> + },
> + .probe_new = sn65dsi_probe,
> + .remove = sn65dsi_remove,
> + .id_table = sn65dsi_i2c_id,
> +};
> +module_i2c_driver(sn65dsi_driver);
> +
> +MODULE_AUTHOR("Jagan Teki ");
> +MODULE_DESCRIPTION("SN65DSI84 DSI to LVDS bridge");
> +MODULE_LICENSE("GPL v2");
> --
> 2.25.1
>
>
--
Michael Nazzareno Trimarchi
Amarula Solutions BV
COO Co-Founder
Cruquiuskade 47 Amsterdam 1018 AM NL
T. +31(0)851119172
M. +39(0)3479132170
[`as] https://www.amarulasolutions.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: dw-mipi-dsi: Find the possible DSI devices
> - struct drm_bridge *bridge;
> > - struct drm_panel *panel;
> > int ret;
> >
> > if (device->lanes > dsi->plat_data->max_data_lanes) {
> > @@ -329,22 +354,14 @@ static int dw_mipi_dsi_host_attach(struct
> > mipi_dsi_host *host,
> > dsi->format = device->format;
> > dsi->mode_flags = device->mode_flags;
> >
> > - ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0,
> > - , );
> > - if (ret)
> > - return ret;
> > + if (!dsi->device_found) {
> > + ret = dw_mipi_dsi_panel_or_bridge(dsi, host->dev->of_node);
> > + if (ret)
> > + return ret;
> >
> > - if (panel) {
> > - bridge = drm_panel_bridge_add_typed(panel,
> > - DRM_MODE_CONNECTOR_DSI);
> > - if (IS_ERR(bridge))
> > - return PTR_ERR(bridge);
> > + dsi->device_found = true;
> > }
> >
> > - dsi->panel_bridge = bridge;
> > -
> > - drm_bridge_add(>bridge);
> > -
> > if (pdata->host_ops && pdata->host_ops->attach) {
> > ret = pdata->host_ops->attach(pdata->priv_data, device);
> > if (ret < 0)
> > @@ -999,6 +1016,16 @@ static int dw_mipi_dsi_bridge_attach(struct
> > drm_bridge *bridge,
> > /* Set the encoder type as caller does not know it */
> > bridge->encoder->encoder_type = DRM_MODE_ENCODER_DSI;
> >
> > + if (!dsi->device_found) {
> > + int ret;
> > +
> > + ret = dw_mipi_dsi_panel_or_bridge(dsi, dsi->dev->of_node);
> > + if (ret)
> > + return ret;
> > +
> > + dsi->device_found = true;
> > + }
> > +
> > /* Attach the panel-bridge to the dsi bridge */
> > return drm_bridge_attach(bridge->encoder, dsi->panel_bridge, bridge,
> >flags);
> > @@ -1181,6 +1208,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> > #ifdef CONFIG_OF
> > dsi->bridge.of_node = pdev->dev.of_node;
> > #endif
> > + drm_bridge_add(>bridge);
> >
> > return dsi;
> > }
> > --
> > 2.25.1
> >
> >
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [RFC PATCH] drm/panel: ilitek-ili9881d: add support for Wanchanglong W552946ABA panel
MMAND_INSTR(0xAA, 0x29),
> > + ILI9881C_COMMAND_INSTR(0xAB, 0x5B),
> > + ILI9881C_COMMAND_INSTR(0xAC, 0x26),
> > + ILI9881C_COMMAND_INSTR(0xAD, 0x28),
> > + ILI9881C_COMMAND_INSTR(0xAE, 0x5C),
> > + ILI9881C_COMMAND_INSTR(0xAF, 0x30),
> > + ILI9881C_COMMAND_INSTR(0xB0, 0x31),
> > + ILI9881C_COMMAND_INSTR(0xB1, 0x2E),
> > + ILI9881C_COMMAND_INSTR(0xB2, 0x32),
> > + ILI9881C_COMMAND_INSTR(0xB3, 0x00),
> > +
> > + ILI9881C_COMMAND_INSTR(0xC0, 0x00),
> > + ILI9881C_COMMAND_INSTR(0xC1, 0x10),
> > + ILI9881C_COMMAND_INSTR(0xC2, 0x1C),
> > + ILI9881C_COMMAND_INSTR(0xC3, 0x13),
> > + ILI9881C_COMMAND_INSTR(0xC4, 0x15),
> > + ILI9881C_COMMAND_INSTR(0xC5, 0x26),
> > + ILI9881C_COMMAND_INSTR(0xC6, 0x1A),
> > + ILI9881C_COMMAND_INSTR(0xC7, 0x1D),
> > + ILI9881C_COMMAND_INSTR(0xC8, 0x67),
> > + ILI9881C_COMMAND_INSTR(0xC9, 0x1C),
> > + ILI9881C_COMMAND_INSTR(0xCA, 0x29),
> > + ILI9881C_COMMAND_INSTR(0xCB, 0x5B),
> > + ILI9881C_COMMAND_INSTR(0xCC, 0x26),
> > + ILI9881C_COMMAND_INSTR(0xCD, 0x28),
> > + ILI9881C_COMMAND_INSTR(0xCE, 0x5C),
> > + ILI9881C_COMMAND_INSTR(0xCF, 0x30),
> > + ILI9881C_COMMAND_INSTR(0xD0, 0x31),
> > + ILI9881C_COMMAND_INSTR(0xD1, 0x2E),
> > + ILI9881C_COMMAND_INSTR(0xD2, 0x32),
> > + ILI9881C_COMMAND_INSTR(0xD3, 0x00),
> > + ILI9881C_SWITCH_PAGE_INSTR(0),
> > +};
> > +
> > static inline struct ili9881c *panel_to_ili9881c(struct drm_panel *panel)
> > {
> > return container_of(panel, struct ili9881c, panel);
> > @@ -603,6 +811,23 @@ static const struct drm_display_mode
> > k101_im2byl02_default_mode = {
> > .height_mm = 217,
> > };
> >
> > +static const struct drm_display_mode w552946aba_default_mode = {
> > + .clock = 64000,
> > +
> > + .hdisplay = 720,
> > + .hsync_start= 720 + 40,
> > + .hsync_end = 720 + 40 + 10,
> > + .htotal = 720 + 40 + 10 + 40,
> > +
> > + .vdisplay = 1280,
> > + .vsync_start= 1280 + 22,
> > + .vsync_end = 1280 + 22 + 4,
> > + .vtotal = 1280 + 22 + 4 + 11,
> > +
> > + .width_mm = 68,
> > + .height_mm = 121,
> > +};
> > +
> > static int ili9881c_get_modes(struct drm_panel *panel,
> > struct drm_connector *connector)
> > {
> > @@ -670,7 +895,7 @@ static int ili9881c_dsi_probe(struct mipi_dsi_device
> > *dsi)
> >
> > drm_panel_add(>panel);
> >
> > - dsi->mode_flags = MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> > + dsi->mode_flags = ctx->desc->mode_flags;
> > dsi->format = MIPI_DSI_FMT_RGB888;
> > dsi->lanes = 4;
> >
> > @@ -691,17 +916,28 @@ static const struct ili9881c_desc lhr050h41_desc = {
> > .init = lhr050h41_init,
> > .init_length = ARRAY_SIZE(lhr050h41_init),
> > .mode = _default_mode,
> > + .mode_flags = MIPI_DSI_MODE_VIDEO_SYNC_PULSE,
> > };
> >
> > static const struct ili9881c_desc k101_im2byl02_desc = {
> > .init = k101_im2byl02_init,
> > .init_length = ARRAY_SIZE(k101_im2byl02_init),
> > .mode = _im2byl02_default_mode,
> > + .mode_flags = MIPI_DSI_MODE_VIDEO_SYNC_PULSE,
> > +};
> > +
> > +static const struct ili9881c_desc w552946aba_desc = {
> > + .init = w552946ab_init,
> > + .init_length = ARRAY_SIZE(w552946ab_init),
> > + .mode = _default_mode,
> > + .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> > + MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_EOT_PACKET,
I need to adjust this flag. I will rebase on drm-fixes
> > };
> >
> > static const struct of_device_id ili9881c_of_match[] = {
> > { .compatible = "bananapi,lhr050h41", .data = _desc },
> > { .compatible = "feixin,k101-im2byl02", .data = _im2byl02_desc },
> > + { .compatible = "wanchanglong,w552946aba", .data = _desc },
>
> wanchanglong - must be included in
> Documentation/devicetree/bindings/vendor-prefixes.yaml
>
Ok
> w552946aba must be documented in a panel DT schema.
> I assume adding it to
> Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
> will do the trick.
Ok
Michael
>
> Sam
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH 5/5] drm/bridge: dw-mipi-dsi: Fix dsi registration during drm probing
Hi Sam
On Sat, Oct 16, 2021 at 2:25 PM Sam Ravnborg wrote:
>
> Hi Michael,
>
> I fail to follow the logic in this patch.
>
>
> On Sat, Oct 16, 2021 at 10:22:32AM +, Michael Trimarchi wrote:
> > The dsi registration is implemented in rockchip platform driver.
> > The attach can be called before the probe is terminated and therefore
> > we need to be sure that corresponding entry during attach is valid
> >
> > Signed-off-by: Michael Trimarchi
> > ---
> > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 +++-
> > drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12
> > include/drm/bridge/dw_mipi_dsi.h| 2 +-
> > 3 files changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > index e44e18a0112a..44b211be15fc 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > @@ -362,8 +362,14 @@ static int dw_mipi_dsi_host_attach(struct
> > mipi_dsi_host *host,
> > dsi->device_found = true;
> > }
> >
> > + /*
> > + * NOTE: the dsi registration is implemented in
> > + * platform driver, that to say dsi would be exist after
> > + * probe is terminated. The call is done before the end of probe
> > + * so we need to pass the dsi to the platform driver.
> > + */
> > if (pdata->host_ops && pdata->host_ops->attach) {
> > - ret = pdata->host_ops->attach(pdata->priv_data, device);
> > + ret = pdata->host_ops->attach(pdata->priv_data, device, dsi);
> > if (ret < 0)
> > return ret;
> > }
> > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > index a2262bee5aa4..32ddc8642ec1 100644
> > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > @@ -997,7 +997,8 @@ static const struct component_ops
> > dw_mipi_dsi_rockchip_ops = {
> > };
> >
> > static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
> > - struct mipi_dsi_device *device)
> > + struct mipi_dsi_device *device,
> > + struct dw_mipi_dsi *dmd)
> > {
> > struct dw_mipi_dsi_rockchip *dsi = priv_data;
> > struct device *second;
> > @@ -1005,6 +1006,8 @@ static int dw_mipi_dsi_rockchip_host_attach(void
> > *priv_data,
> >
> > mutex_lock(>usage_mutex);
> >
> > + dsi->dmd = dmd;
> > +
> > if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> > DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> > mutex_unlock(>usage_mutex);
> > @@ -1280,6 +1283,7 @@ static int dw_mipi_dsi_rockchip_probe(struct
> > platform_device *pdev)
> > {
> > struct device *dev = >dev;
> > struct device_node *np = dev->of_node;
> > + struct dw_mipi_dsi *dmd;
> > struct dw_mipi_dsi_rockchip *dsi;
> > struct phy_provider *phy_provider;
> > struct resource *res;
> > @@ -1391,9 +1395,9 @@ static int dw_mipi_dsi_rockchip_probe(struct
> > platform_device *pdev)
> > if (IS_ERR(phy_provider))
> > return PTR_ERR(phy_provider);
> >
> > - dsi->dmd = dw_mipi_dsi_probe(pdev, >pdata);
> > - if (IS_ERR(dsi->dmd)) {
> > - ret = PTR_ERR(dsi->dmd);
> > + dmd = dw_mipi_dsi_probe(pdev, >pdata);
> > + if (IS_ERR(dmd)) {
> > + ret = PTR_ERR(dmd);
>
> The memory pointed to by dmd is allocated in dw_mipi_dsi_probe(), but
> the pointer is not saved here.
> We rely on the attach operation to save the dmd pointer.
>
>
> In other words - the attach operation must be called before we call
> dw_mipi_dsi_rockchip_remove(), which uses the dmd member.
>
> This all looks wrong to me - are we papering over some other issue
Ok, it's wrong. I was not expecting that call.Anyway this was my path
on linux-next
dw_mipi_dsi_rockchip_probe
dw_mipi_dsi_probe -->start call
dw_mipi_dsi_rockchip_host_attach <-- this was not able to use dmd
dw_mipi_dsi_probe -> exit from the call
Michael
> here?
>
> Sam
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH 5/5] drm/bridge: dw-mipi-dsi: Fix dsi registration during drm probing
Hi Forget this one I can not replicate. We have another problem 9862.010474] Hardware name: Rockchip PX30 EVB (DT) [ 9862.015738] Call trace: [ 9862.018471] dump_backtrace+0x0/0x19c [ 9862.022586] show_stack+0x1c/0x70 [ 9862.026305] dump_stack_lvl+0x68/0x84 [ 9862.030408] dump_stack+0x1c/0x38 [ 9862.034125] ili9881c_unprepare+0x1c/0x4c [ 9862.038619] drm_panel_unprepare+0x2c/0x4c [ 9862.043212] panel_bridge_post_disable+0x18/0x24 [ 9862.048390] dw_mipi_dsi_bridge_post_disable+0x3c/0xf0 [ 9862.054153] drm_atomic_bridge_chain_post_disable+0x8c/0xd0 [ 9862.060399] disable_outputs+0x120/0x31c [ 9862.064785] drm_atomic_helper_commit_tail_rpm+0x28/0xa0 [ 9862.070734] commit_tail+0xa4/0x184 [ 9862.074642] drm_atomic_helper_commit+0x164/0x37c [ 9862.079902] drm_atomic_commit+0x50/0x60 [ 9862.084298] drm_atomic_helper_disable_all+0x1fc/0x210 [ 9862.090053] drm_atomic_helper_shutdown+0x80/0x130 [ 9862.095419] rockchip_drm_platform_shutdown+0x1c/0x30 [ 9862.101081] platform_shutdown+0x28/0x40 [ 9862.105477] device_shutdown+0x15c/0x330 [ 9862.109877] __do_sys_reboot+0x214/0x294 [ 9862.114273] __arm64_sys_reboot+0x28/0x3c [ 9862.118765] invoke_syscall+0x48/0x114 [ 9862.122969] el0_svc_common.constprop.0+0x44/0xec [ 9862.128241] do_el0_svc+0x28/0x90 [ 9862.131958] el0_svc+0x20/0x60 [ 9862.135381] el0t_64_sync_handler+0x1a8/0x1b0 [ 9862.140263] el0t_64_sync+0x1a0/0x1a4 [ 9862.145769] CPU: 0 PID: 1 Comm: systemd-shutdow Tainted: GW 5.15.0-rc5 #1 [ 9862.154957] Hardware name: Rockchip PX30 EVB (DT) [ 9862.160221] Call trace: [ 9862.162954] dump_backtrace+0x0/0x19c [ 9862.167068] show_stack+0x1c/0x70 [ 9862.170787] dump_stack_lvl+0x68/0x84 [ 9862.174895] dump_stack+0x1c/0x38 [ 9862.178611] ili9881c_unprepare+0x1c/0x4c [ 9862.183103] drm_panel_unprepare+0x2c/0x4c [ 9862.187695] panel_bridge_post_disable+0x18/0x24 [ 9862.192872] drm_atomic_bridge_chain_post_disable+0x8c/0xd0 [ 9862.199117] disable_outputs+0x120/0x31c [ 9862.203512] drm_atomic_helper_commit_tail_rpm+0x28/0xa0 [ 9862.209461] commit_tail+0xa4/0x184 [ 9862.213368] drm_atomic_helper_commit+0x164/0x37c [ 9862.218629] drm_atomic_commit+0x50/0x60 [ 9862.223025] drm_atomic_helper_disable_all+0x1fc/0x210 [ 9862.228781] drm_atomic_helper_shutdown+0x80/0x130 [ 9862.234147] rockchip_drm_platform_shutdown+0x1c/0x30 [ 9862.239810] platform_shutdown+0x28/0x40 [ 9862.244205] device_shutdown+0x15c/0x330 [ 9862.248603] __do_sys_reboot+0x214/0x294 [ 9862.253000] __arm64_sys_reboot+0x28/0x3c [ 9862.257492] invoke_syscall+0x48/0x114 [ 9862.261696] el0_svc_common.constprop.0+0x44/0xec [ 9862.266970] do_el0_svc+0x28/0x90 [ 9862.270687] el0_svc+0x20/0x60 [ 9862.274111] el0t_64_sync_handler+0x1a8/0x1b0 [ 9862.278992] el0t_64_sync+0x1a0/0x1a4 [ 9862.283296] [ cut here ] [ 9862.288490] unbalanced disables for vcc3v3_lcd [ 9862.293555] WARNING: CPU: 0 PID: 1 at drivers/regulator/core.c:2851 _regulator_disable+0xd4/0x190 The panel can be disable two times. /* * TODO Only way found to call panel-bridge post_disable & * panel unprepare before the dsi "final" disable... * This needs to be fixed in the drm_bridge framework and the API * needs to be updated to manage our own call chains... */ if (dsi->panel_bridge->funcs->post_disable) dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge); Is this comment relevant? Michael On Sat, Oct 16, 2021 at 3:32 PM Michael Nazzareno Trimarchi wrote: > > Hi Sam > > On Sat, Oct 16, 2021 at 2:25 PM Sam Ravnborg wrote: > > > > Hi Michael, > > > > I fail to follow the logic in this patch. > > > > > > On Sat, Oct 16, 2021 at 10:22:32AM +, Michael Trimarchi wrote: > > > The dsi registration is implemented in rockchip platform driver. > > > The attach can be called before the probe is terminated and therefore > > > we need to be sure that corresponding entry during attach is valid > > > > > > Signed-off-by: Michael Trimarchi > > > --- > > > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 +++- > > > drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 > > > include/drm/bridge/dw_mipi_dsi.h| 2 +- > > > 3 files changed, 16 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > > index e44e18a0112a..44b211be15fc 100644 > > > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > > @@ -362,8 +362,14 @@ static int dw_mipi_dsi_host_attach(struct > > > mipi_dsi_host *host, > > > dsi->device_found = true; >
Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
gt; > > > > bindings and look for a panel or bridge not only through the
> > > > > > > > > OF graph,
> > > > > > > > > but also on the child nodes
> > > > > > > >
> > > > > > > > Okay. I need to check this.
> > > > > > >
> > > > > > > devm_drm_of_get_bridge is not working with legacy binding like
> > > > > > > the one
> > > > > > > used in sun6i dsi
> > > > > >
> > > > > > There's nothing legacy about it.
> > > > >
> > > > > What I'm mean legacy here with current binding used in sun6i-dsi like
> > > > > this.
> > > > >
> > > > > {
> > > > > vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> > > > > status = "okay";
> > > > >
> > > > > panel@0 {
> > > > >compatible = "bananapi,s070wv20-ct16-icn6211";
> > > > >reg = <0>;
> > > > >reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /*
> > > > > LCD-RST: PL5 */
> > > > > enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /*
> > > > > LCD-PWR-EN: PB7 */
> > > > > backlight = <>;
> > > > > };
> > > > > };
> > > >
> > > > Yes, I know, it's the generic DSI binding. It's still not legacy.
> > > >
> > > > > devm_drm_of_get_bridge cannot find the device with above binding and
> > > > > able to find the device with below binding.
> > > > >
> > > > > {
> > > > >vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> > > > >status = "okay";
> > > > >
> > > > > ports {
> > > > > #address-cells = <1>;
> > > > > #size-cells = <0>;
> > > > >
> > > > >dsi_out: port@0 {
> > > > >reg = <0>;
> > > > >
> > > > > dsi_out_bridge: endpoint {
> > > > > remote-endpoint = <_out_dsi>;
> > > > > };
> > > > >};
> > > > > };
> > > > >
> > > > > panel@0 {
> > > > > compatible = "bananapi,s070wv20-ct16-icn6211";
> > > > > reg = <0>;
> > > > > reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST:
> > > > > PL5 */
> > > > > enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /*
> > > > > LCD-PWR-EN: PB7 */
> > > > > backlight = <>;
> > > > >
> > > > > port {
> > > > > bridge_out_dsi: endpoint {
> > > > > remote-endpoint = <_out_bridge>;
> > > > > };
> > > > > };
> > > > >};
> > > > > };
> > > >
> > > > Yes, I know, and that's because ...
> > >
> > > Okay. I will use find panel and bridge separately instead of
> > > devm_drm_of_get_bridge in version patches.
> >
> > That's not been my point, at all?
> >
> > I mean, that whole discussion has been because you shouldn't do that.
> >
> > > >
> > > > > >
> > > > > > > https://patchwork.kernel.org/project/dri-devel/patch/20211122065223.88059-6-ja...@amarulasolutions.com/
> > > > > > >
> > > > > > > dsi->next_bridge = devm_drm_of_get_bridge(dsi->dev,
> > > > > > > dsi->dev->of_node, 0, 0);
> > > > > > > if (IS_ERR(dsi->next_bridge))
> > > > > > >return PTR_ERR(dsi->next_bridge);
> > > > > > >
> > > > > > > It is only working if we have ports on the pipeline, something
> > > > > > > like this
> > > > > > > https://patchwork.kernel.org/project/dri-devel/patch/20210214194102.126146-8-ja...@amarulasolutions.com/
> > > > > > >
> > > > > > > Please have a look and let me know if I miss anything?
> > > > > >
> > > > > > Yes, you're missing the answer you quoted earlier:
> > > > >
> > > > > Yes, I'm trying to resolve the comment one after another. Will get
> > > > > back.
> > > >
> > > > ... You've ignored that comment.
> > >
> > > Not understand which comment you mean. There are few about bridge
> > > conversion details, I will send my comments.
> >
> > The one that got quoted there and you removed. For reference:
> >
> > > Then drm_of_find_panel_or_bridge needs to be adjusted to handle the DSI
> > > bindings and look for a panel or bridge not only through the OF graph,
> > > but also on the child nodes
> >
> > devm_drm_of_get_bridge uses drm_of_find_panel_or_bridge under the hood,
> > so of course it won't find it if drm_of_find_panel_or_bridge doesn't.
> > You need to modify drm_of_find_panel_or_bridge to also look for child
> > devices and see if there's a panel or bridge registered for that child
> > node. Then devm_drm_of_get_bridge will work as you intend it to.
>
> Got it now, I will make necessary changes.
>
> Thanks,
> Jagan.
>
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH 0/5] Add support for Wanchanglong panel used in px30-evb v11
Hi Sam On Sat, Oct 16, 2021 at 2:27 PM Sam Ravnborg wrote: > > Hi Michael, > > On Sat, Oct 16, 2021 at 10:22:27AM +, Michael Trimarchi wrote: > > This patch series add support for W552946ABA panel. This panel is used > > in px30-evb v11. All the patches can be applied on top of drm-fixes > > branch. The last patch is suppose to fix a race when the panel is built > > in. Tested on px30 evb > > > > Michael Trimarchi (5): > > dt-bindings: vendor-prefix: add Wanchanglong Electronics Technology > > drm/panel: ilitek-ili9881d: add support for Wanchanglong W552946ABA > > panel > > dt-bindings: ili9881c: add compatible string for Wanchanglong > > w552946aba > > drm/panel: ilitek-ili9881c: Make gpio-reset optional > The four patches has been applied to drm-misc-next. > I sent another fix on the same panel. Are those patches queued on some tree? > > drm/bridge: dw-mipi-dsi: Fix dsi registration during drm probing > This patch looks like it does not belong in this series. > Anyway - commented on it as I did not understand the code. > > Sam Michael
Re: [RFC PATCH 00/17] drm: bridge: Samsung MIPI DSIM bridge
Hi Tim On Thu, Dec 9, 2021 at 5:40 PM Tim Harvey wrote: > > On Thu, Dec 9, 2021 at 12:36 AM Michael Nazzareno Trimarchi > wrote: > > > > Hi Tim > > > > On Tue, Oct 5, 2021 at 11:43 PM Tim Harvey wrote: > > > > > > On Sun, Jul 25, 2021 at 10:14 AM Jagan Teki > > > wrote: > > > > > > > > Hi Sam, > > > > > > > > On Sun, Jul 25, 2021 at 10:35 PM Sam Ravnborg wrote: > > > > > > > > > > Hi Jagan, > > > > > > > > > > On Sun, Jul 04, 2021 at 02:32:13PM +0530, Jagan Teki wrote: > > > > > > This series supports common bridge support for Samsung MIPI DSIM > > > > > > which is used in Exynos and i.MX8MM SoC's. > > > > > > > > > > > > The final bridge supports both the Exynos and i.MX8MM DSI devices. > > > > > > > > > > > > Right now bridge offers two sets of implementations. > > > > > > > > > > > > A. With component_ops and exynos specific code exclusively for > > > > > >exynos dsi drivers and it's legacy bindings. > > > > > > > > > > > > B. Without componenet_ops for newly implemented bridges and its > > > > > >users like i.MX8MM. > > > > > > > > > > > > The future plan is to fix the implementation A) by dropping > > > > > > component_ops and fixing exynos specific code in order to make > > > > > > the bridge more mature to use and the same is mentioned in > > > > > > drivers TODO. > > > > > > > > > > > > Patch 0001 - 0006: Bridge conversion > > > > > > Patch 0007 - 0017: Samsung MIPI DSIM bridge fixes, additions > > > > > > > > > > > > Tested in Engicam i.Core MX8M Mini SoM. > > > > > > > > > > > > Anyone interest, please have a look on this repo > > > > > > https://github.com/openedev/linux/tree/070421-imx8mm-dsim > > > > > > > > > > > > Would appreciate anyone from the exynos team to test it on > > > > > > the exynos platform? > > > > > > > > > > > > Any inputs? > > > > > > > > > > I really like where you are headign with this! > > > > > No testing - sorry. But I will try to provide a bit of feedback on the > > > > > individual patches. > > > > > > > > > > I hope you find a way to move forward with this. > > > > > > > > Thanks for the response. > > > > > > > > We have found some issues with Bridge conversion on existing exynos > > > > drivers. The component based DSI drivers(like exynos) are difficult to > > > > attach if it involves kms hotplug. kms hotplug would require drm > > > > pointer and that pointer would only available after the bind call > > > > finishes. But the bridge attach in bind call will defer till it find > > > > the attached bridge. > > > > > > > > Right now I'm trying to find the proper way to attach the bridges for > > > > component based DSI drivers which involves kms hot-plug. > > > > > > > > If you have any ideas on this, please let me know. > > > > > > > > > > Jagan, > > > > > > How is your progress on this series? Looking at your repo it looks > > > like you've rebased on top of 5.13-rc3 in your 070121-imx8mm-dsim > > > branch but you've got a lot of things there that are likely not > > > related to this series? > > > > I have a bit of work on those patches and tested on imx8mn. Basically: > > > > - add the dsi timing calculation > > - change few difference with samsung bridge > > - fix crashes of my dsi panels > > - compare with NXP driver the final results > > > > I found that I have one problem that gives me some instability. In the > > NXP original driver the panel needs to be > > enabled in bridge_enable before out the standby. If I understand > > correctly, our standby should be done after. > > I would like to have some feedback and help and testing on other > > boards/devices and some suggestions on how to handle > > some of the differences. Another big problem is etnavi that is not stable > > > > Michael, > > Where can I find your patches? > I will push on some tree and share > What do you mean by etnaviv not being stable? > &
Re: [RFC PATCH 00/17] drm: bridge: Samsung MIPI DSIM bridge
Hi Tim On Tue, Oct 5, 2021 at 11:43 PM Tim Harvey wrote: > > On Sun, Jul 25, 2021 at 10:14 AM Jagan Teki > wrote: > > > > Hi Sam, > > > > On Sun, Jul 25, 2021 at 10:35 PM Sam Ravnborg wrote: > > > > > > Hi Jagan, > > > > > > On Sun, Jul 04, 2021 at 02:32:13PM +0530, Jagan Teki wrote: > > > > This series supports common bridge support for Samsung MIPI DSIM > > > > which is used in Exynos and i.MX8MM SoC's. > > > > > > > > The final bridge supports both the Exynos and i.MX8MM DSI devices. > > > > > > > > Right now bridge offers two sets of implementations. > > > > > > > > A. With component_ops and exynos specific code exclusively for > > > >exynos dsi drivers and it's legacy bindings. > > > > > > > > B. Without componenet_ops for newly implemented bridges and its > > > >users like i.MX8MM. > > > > > > > > The future plan is to fix the implementation A) by dropping > > > > component_ops and fixing exynos specific code in order to make > > > > the bridge more mature to use and the same is mentioned in > > > > drivers TODO. > > > > > > > > Patch 0001 - 0006: Bridge conversion > > > > Patch 0007 - 0017: Samsung MIPI DSIM bridge fixes, additions > > > > > > > > Tested in Engicam i.Core MX8M Mini SoM. > > > > > > > > Anyone interest, please have a look on this repo > > > > https://github.com/openedev/linux/tree/070421-imx8mm-dsim > > > > > > > > Would appreciate anyone from the exynos team to test it on > > > > the exynos platform? > > > > > > > > Any inputs? > > > > > > I really like where you are headign with this! > > > No testing - sorry. But I will try to provide a bit of feedback on the > > > individual patches. > > > > > > I hope you find a way to move forward with this. > > > > Thanks for the response. > > > > We have found some issues with Bridge conversion on existing exynos > > drivers. The component based DSI drivers(like exynos) are difficult to > > attach if it involves kms hotplug. kms hotplug would require drm > > pointer and that pointer would only available after the bind call > > finishes. But the bridge attach in bind call will defer till it find > > the attached bridge. > > > > Right now I'm trying to find the proper way to attach the bridges for > > component based DSI drivers which involves kms hot-plug. > > > > If you have any ideas on this, please let me know. > > > > Jagan, > > How is your progress on this series? Looking at your repo it looks > like you've rebased on top of 5.13-rc3 in your 070121-imx8mm-dsim > branch but you've got a lot of things there that are likely not > related to this series? I have a bit of work on those patches and tested on imx8mn. Basically: - add the dsi timing calculation - change few difference with samsung bridge - fix crashes of my dsi panels - compare with NXP driver the final results I found that I have one problem that gives me some instability. In the NXP original driver the panel needs to be enabled in bridge_enable before out the standby. If I understand correctly, our standby should be done after. I would like to have some feedback and help and testing on other boards/devices and some suggestions on how to handle some of the differences. Another big problem is etnavi that is not stable Michael > > Best regards, > > Tim > > -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com
Re: [PATCH] drm/panel: ilitek-ili9881c: Avoid unbalance prepare/unprepare
Hi Dave
On Thu, Dec 9, 2021 at 6:58 PM Dave Stevenson
wrote:
>
> Hi Michael
>
> On Thu, 9 Dec 2021 at 16:58, Michael Nazzareno Trimarchi
> wrote:
> >
> > Hi all
> >
> > On Sat, Oct 16, 2021 at 4:58 PM Michael Trimarchi
> > wrote:
> > >
> > > All the panel driver check the fact that their prepare/unprepare
> > > call was already called. It's not an ideal solution but fix
> > > for now the problem on ili9881c
> > >
> > > [ 9862.283296] [ cut here ]
> > > [ 9862.288490] unbalanced disables for vcc3v3_lcd
> > > [ 9862.293555] WARNING: CPU: 0 PID: 1 at drivers/regulator/core.c:2851
> > > _regulator_disable+0xd4/0x190
> > >
> > > from:
> > >
> > > [ 9862.038619] drm_panel_unprepare+0x2c/0x4c
> > > [ 9862.043212] panel_bridge_post_disable+0x18/0x24
> > > [ 9862.048390] dw_mipi_dsi_bridge_post_disable+0x3c/0xf0
> > > [ 9862.054153] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
> > >
> > > and:
> > >
> > > [ 9862.183103] drm_panel_unprepare+0x2c/0x4c
> > > [ 9862.187695] panel_bridge_post_disable+0x18/0x24
> > > [ 9862.192872] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
> > > [ 9862.199117] disable_outputs+0x120/0x31c
>
> This is down to the dw-mipi-dsi driver calling the post_disable hook
> explicitly at [1], but then also allowing the framework to call it.
> The explicit call is down to limitations in the DSI support, so we
> can't control the DSI host state to a fine enough degree (an ongoing
> discussion [2] [3]). There shouldn't be a need to handle mismatched
> calling in individual panel drivers.
>
> Dave
>
> [1]
> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L894
> [2] https://lists.freedesktop.org/archives/dri-devel/2021-November/332060.html
> [3] https://lists.freedesktop.org/archives/dri-devel/2021-December/334007.html
I'm in the second case. I need to enable HS mode after the panel is
initialized. Time to time I have timeout
on dsi command or I have wrong panel initialization. So I explicit call from
the bridge but I understand that is not correct in the design point of view.
So this patch can not be queued because it's a known problem that
people are discussing
Michael
>
>
> > > Signed-off-by: Michael Trimarchi
> > > ---
> > > drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++
> > > 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > > b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > > index 103a16018975..f75eecb0e65c 100644
> > > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > > @@ -52,6 +52,8 @@ struct ili9881c {
> > >
> > > struct regulator*power;
> > > struct gpio_desc*reset;
> > > +
> > > + boolprepared;
> > > };
> > >
> >
> > I found that this can be a general problem. Should not mandatory to
> > track panel status
> >
> > DRM_PANEL_PREPARED
> > DRM_PANEL_ENABLED
> >
> > Michael
> > > #define ILI9881C_SWITCH_PAGE_INSTR(_page) \
> > > @@ -707,6 +709,10 @@ static int ili9881c_prepare(struct drm_panel *panel)
> > > unsigned int i;
> > > int ret;
> > >
> > > + /* Preparing when already prepared is a no-op */
> > > + if (ctx->prepared)
> > > + return 0;
> > > +
> > > /* Power the panel */
> > > ret = regulator_enable(ctx->power);
> > > if (ret)
> > > @@ -745,6 +751,8 @@ static int ili9881c_prepare(struct drm_panel *panel)
> > > if (ret)
> > > return ret;
> > >
> > > + ctx->prepared = true;
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -770,10 +778,16 @@ static int ili9881c_unprepare(struct drm_panel
> > > *panel)
> > > {
> > > struct ili9881c *ctx = panel_to_ili9881c(panel);
> > >
> > > + /* Unpreparing when already unprepared is a no-op */
> > > + if (!ctx->prepared)
> > > + return 0;
> > > +
> > > mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> > > regulator_disable(ctx->power);
> > > gpiod_set_value(ctx->reset, 1);
> > >
> > > + ctx->prepared = false;
> > > +
> > > return 0;
> > > }
> > >
> > > --
> > > 2.25.1
> > >
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > mich...@amarulasolutions.com
> > __
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > i...@amarulasolutions.com
> > www.amarulasolutions.com
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH] drm/panel: ilitek-ili9881c: Avoid unbalance prepare/unprepare
Hi all
On Sat, Oct 16, 2021 at 4:58 PM Michael Trimarchi
wrote:
>
> All the panel driver check the fact that their prepare/unprepare
> call was already called. It's not an ideal solution but fix
> for now the problem on ili9881c
>
> [ 9862.283296] [ cut here ]
> [ 9862.288490] unbalanced disables for vcc3v3_lcd
> [ 9862.293555] WARNING: CPU: 0 PID: 1 at drivers/regulator/core.c:2851
> _regulator_disable+0xd4/0x190
>
> from:
>
> [ 9862.038619] drm_panel_unprepare+0x2c/0x4c
> [ 9862.043212] panel_bridge_post_disable+0x18/0x24
> [ 9862.048390] dw_mipi_dsi_bridge_post_disable+0x3c/0xf0
> [ 9862.054153] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
>
> and:
>
> [ 9862.183103] drm_panel_unprepare+0x2c/0x4c
> [ 9862.187695] panel_bridge_post_disable+0x18/0x24
> [ 9862.192872] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
> [ 9862.199117] disable_outputs+0x120/0x31c
>
> Signed-off-by: Michael Trimarchi
> ---
> drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> index 103a16018975..f75eecb0e65c 100644
> --- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> @@ -52,6 +52,8 @@ struct ili9881c {
>
> struct regulator*power;
> struct gpio_desc*reset;
> +
> + boolprepared;
> };
>
I found that this can be a general problem. Should not mandatory to
track panel status
DRM_PANEL_PREPARED
DRM_PANEL_ENABLED
Michael
> #define ILI9881C_SWITCH_PAGE_INSTR(_page) \
> @@ -707,6 +709,10 @@ static int ili9881c_prepare(struct drm_panel *panel)
> unsigned int i;
> int ret;
>
> + /* Preparing when already prepared is a no-op */
> + if (ctx->prepared)
> + return 0;
> +
> /* Power the panel */
> ret = regulator_enable(ctx->power);
> if (ret)
> @@ -745,6 +751,8 @@ static int ili9881c_prepare(struct drm_panel *panel)
> if (ret)
> return ret;
>
> + ctx->prepared = true;
> +
> return 0;
> }
>
> @@ -770,10 +778,16 @@ static int ili9881c_unprepare(struct drm_panel *panel)
> {
> struct ili9881c *ctx = panel_to_ili9881c(panel);
>
> + /* Unpreparing when already unprepared is a no-op */
> + if (!ctx->prepared)
> + return 0;
> +
> mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> regulator_disable(ctx->power);
> gpiod_set_value(ctx->reset, 1);
>
> + ctx->prepared = false;
> +
> return 0;
> }
>
> --
> 2.25.1
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [RFC PATCH 00/17] drm: bridge: Samsung MIPI DSIM bridge
Hi On Thu, Dec 9, 2021 at 9:24 PM Lucas Stach wrote: > > Am Donnerstag, dem 09.12.2021 um 18:09 +0100 schrieb Michael Nazzareno > Trimarchi: > > Hi Tim > > > > On Thu, Dec 9, 2021 at 5:40 PM Tim Harvey wrote: > > > > > > On Thu, Dec 9, 2021 at 12:36 AM Michael Nazzareno Trimarchi > > > wrote: > > > > > > > > Hi Tim > > > > > > > > On Tue, Oct 5, 2021 at 11:43 PM Tim Harvey > > > > wrote: > > > > > > > > > > On Sun, Jul 25, 2021 at 10:14 AM Jagan Teki > > > > > wrote: > > > > > > > > > > > > Hi Sam, > > > > > > > > > > > > On Sun, Jul 25, 2021 at 10:35 PM Sam Ravnborg > > > > > > wrote: > > > > > > > > > > > > > > Hi Jagan, > > > > > > > > > > > > > > On Sun, Jul 04, 2021 at 02:32:13PM +0530, Jagan Teki wrote: > > > > > > > > This series supports common bridge support for Samsung MIPI DSIM > > > > > > > > which is used in Exynos and i.MX8MM SoC's. > > > > > > > > > > > > > > > > The final bridge supports both the Exynos and i.MX8MM DSI > > > > > > > > devices. > > > > > > > > > > > > > > > > Right now bridge offers two sets of implementations. > > > > > > > > > > > > > > > > A. With component_ops and exynos specific code exclusively for > > > > > > > >exynos dsi drivers and it's legacy bindings. > > > > > > > > > > > > > > > > B. Without componenet_ops for newly implemented bridges and its > > > > > > > >users like i.MX8MM. > > > > > > > > > > > > > > > > The future plan is to fix the implementation A) by dropping > > > > > > > > component_ops and fixing exynos specific code in order to make > > > > > > > > the bridge more mature to use and the same is mentioned in > > > > > > > > drivers TODO. > > > > > > > > > > > > > > > > Patch 0001 - 0006: Bridge conversion > > > > > > > > Patch 0007 - 0017: Samsung MIPI DSIM bridge fixes, additions > > > > > > > > > > > > > > > > Tested in Engicam i.Core MX8M Mini SoM. > > > > > > > > > > > > > > > > Anyone interest, please have a look on this repo > > > > > > > > https://github.com/openedev/linux/tree/070421-imx8mm-dsim > > > > > > > > > > > > > > > > Would appreciate anyone from the exynos team to test it on > > > > > > > > the exynos platform? > > > > > > > > > > > > > > > > Any inputs? > > > > > > > > > > > > > > I really like where you are headign with this! > > > > > > > No testing - sorry. But I will try to provide a bit of feedback > > > > > > > on the > > > > > > > individual patches. > > > > > > > > > > > > > > I hope you find a way to move forward with this. > > > > > > > > > > > > Thanks for the response. > > > > > > > > > > > > We have found some issues with Bridge conversion on existing exynos > > > > > > drivers. The component based DSI drivers(like exynos) are difficult > > > > > > to > > > > > > attach if it involves kms hotplug. kms hotplug would require drm > > > > > > pointer and that pointer would only available after the bind call > > > > > > finishes. But the bridge attach in bind call will defer till it find > > > > > > the attached bridge. > > > > > > > > > > > > Right now I'm trying to find the proper way to attach the bridges > > > > > > for > > > > > > component based DSI drivers which involves kms hot-plug. > > > > > > > > > > > > If you have any ideas on this, please let me know. > > > > > > > > > > > > > > > > Jagan, > > > > > > > > > > How is your progress on this series? Looking at your repo it looks > > > > > like you've rebas
Re: [PATCH] drm/panel: ilitek-ili9881c: Avoid unbalance prepare/unprepare
Hi Dave
some questions below
On Thu, Dec 9, 2021 at 7:10 PM Michael Nazzareno Trimarchi
wrote:
>
> Hi Dave
>
> On Thu, Dec 9, 2021 at 6:58 PM Dave Stevenson
> wrote:
> >
> > Hi Michael
> >
> > On Thu, 9 Dec 2021 at 16:58, Michael Nazzareno Trimarchi
> > wrote:
> > >
> > > Hi all
> > >
> > > On Sat, Oct 16, 2021 at 4:58 PM Michael Trimarchi
> > > wrote:
> > > >
> > > > All the panel driver check the fact that their prepare/unprepare
> > > > call was already called. It's not an ideal solution but fix
> > > > for now the problem on ili9881c
> > > >
> > > > [ 9862.283296] [ cut here ]
> > > > [ 9862.288490] unbalanced disables for vcc3v3_lcd
> > > > [ 9862.293555] WARNING: CPU: 0 PID: 1 at drivers/regulator/core.c:2851
> > > > _regulator_disable+0xd4/0x190
> > > >
> > > > from:
> > > >
> > > > [ 9862.038619] drm_panel_unprepare+0x2c/0x4c
> > > > [ 9862.043212] panel_bridge_post_disable+0x18/0x24
> > > > [ 9862.048390] dw_mipi_dsi_bridge_post_disable+0x3c/0xf0
> > > > [ 9862.054153] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
> > > >
> > > > and:
> > > >
> > > > [ 9862.183103] drm_panel_unprepare+0x2c/0x4c
> > > > [ 9862.187695] panel_bridge_post_disable+0x18/0x24
> > > > [ 9862.192872] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
> > > > [ 9862.199117] disable_outputs+0x120/0x31c
> >
> > This is down to the dw-mipi-dsi driver calling the post_disable hook
> > explicitly at [1], but then also allowing the framework to call it.
> > The explicit call is down to limitations in the DSI support, so we
> > can't control the DSI host state to a fine enough degree (an ongoing
> > discussion [2] [3]). There shouldn't be a need to handle mismatched
> > calling in individual panel drivers.
> >
> > Dave
> >
> > [1]
> > https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L894
> > [2]
> > https://lists.freedesktop.org/archives/dri-devel/2021-November/332060.html
> > [3]
> > https://lists.freedesktop.org/archives/dri-devel/2021-December/334007.html
>
> I'm in the second case. I need to enable HS mode after the panel is
> initialized. Time to time I have timeout
> on dsi command or I have wrong panel initialization. So I explicit call from
> the bridge but I understand that is not correct in the design point of view.
>
> So this patch can not be queued because it's a known problem that
> people are discussing
>
Author: Michael Trimarchi
Date: Thu Dec 9 15:45:48 2021 +0100
drm: bridge: samsung-dsim: Enable panel/bridge before exist from standby
We need to exist from standby as last operation to have a proper video
working. This code implement the same code was before the bridge
migration
Signed-off-by: Michael Trimarchi
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 654851edbd9b..21265ae80022 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1838,6 +1838,7 @@ static void samsung_dsim_atomic_enable(struct
drm_bridge *bridge,
struct drm_bridge_state
*old_bridge_state)
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+ struct drm_atomic_state old_state;
int ret;
if (dsi->state & DSIM_STATE_ENABLED)
@@ -1859,6 +1860,9 @@ static void samsung_dsim_atomic_enable(struct
drm_bridge *bridge,
}
samsung_dsim_set_display_mode(dsi);
+
+ drm_atomic_bridge_chain_enable(dsi->out_bridge, _state);
+
samsung_dsim_set_display_enable(dsi, true);
dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
Right now I'm doing this to enable the change. I must change the panel
to avoid double enabled
I have some questions:
- the chain is an element (bridge/panel) linked together via some
connector (I hope I understand) when I enable
a bridge chain, all the elements should move from some status to
another. If we mark them already this should
not avoid that one element can be enabled two times? An element that
sources two other elements should for instance
receive the enable from two times before switching on.
Michael
> Michael
>
> >
> >
> > > > Signed-off-by: Michael Trimarchi
> > > > ---
> > > > drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++
> > > > 1 file changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu
Re: [PATCH] drm/panel: ilitek-ili9881c: Avoid unbalance prepare/unprepare
Hi
On Fri, Dec 10, 2021 at 4:48 PM Dave Stevenson
wrote:
>
> Hi Michael
>
> On Fri, 10 Dec 2021 at 09:05, Michael Nazzareno Trimarchi
> wrote:
> >
> > Hi Dave
> >
> > some questions below
> >
> > On Thu, Dec 9, 2021 at 7:10 PM Michael Nazzareno Trimarchi
> > wrote:
> > >
> > > Hi Dave
> > >
> > > On Thu, Dec 9, 2021 at 6:58 PM Dave Stevenson
> > > wrote:
> > > >
> > > > Hi Michael
> > > >
> > > > On Thu, 9 Dec 2021 at 16:58, Michael Nazzareno Trimarchi
> > > > wrote:
> > > > >
> > > > > Hi all
> > > > >
> > > > > On Sat, Oct 16, 2021 at 4:58 PM Michael Trimarchi
> > > > > wrote:
> > > > > >
> > > > > > All the panel driver check the fact that their prepare/unprepare
> > > > > > call was already called. It's not an ideal solution but fix
> > > > > > for now the problem on ili9881c
> > > > > >
> > > > > > [ 9862.283296] [ cut here ]
> > > > > > [ 9862.288490] unbalanced disables for vcc3v3_lcd
> > > > > > [ 9862.293555] WARNING: CPU: 0 PID: 1 at
> > > > > > drivers/regulator/core.c:2851
> > > > > > _regulator_disable+0xd4/0x190
> > > > > >
> > > > > > from:
> > > > > >
> > > > > > [ 9862.038619] drm_panel_unprepare+0x2c/0x4c
> > > > > > [ 9862.043212] panel_bridge_post_disable+0x18/0x24
> > > > > > [ 9862.048390] dw_mipi_dsi_bridge_post_disable+0x3c/0xf0
> > > > > > [ 9862.054153] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
> > > > > >
> > > > > > and:
> > > > > >
> > > > > > [ 9862.183103] drm_panel_unprepare+0x2c/0x4c
> > > > > > [ 9862.187695] panel_bridge_post_disable+0x18/0x24
> > > > > > [ 9862.192872] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
> > > > > > [ 9862.199117] disable_outputs+0x120/0x31c
> > > >
> > > > This is down to the dw-mipi-dsi driver calling the post_disable hook
> > > > explicitly at [1], but then also allowing the framework to call it.
> > > > The explicit call is down to limitations in the DSI support, so we
> > > > can't control the DSI host state to a fine enough degree (an ongoing
> > > > discussion [2] [3]). There shouldn't be a need to handle mismatched
> > > > calling in individual panel drivers.
> > > >
> > > > Dave
> > > >
> > > > [1]
> > > > https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L894
> > > > [2]
> > > > https://lists.freedesktop.org/archives/dri-devel/2021-November/332060.html
> > > > [3]
> > > > https://lists.freedesktop.org/archives/dri-devel/2021-December/334007.html
> > >
> > > I'm in the second case. I need to enable HS mode after the panel is
> > > initialized. Time to time I have timeout
> > > on dsi command or I have wrong panel initialization. So I explicit call
> > > from
> > > the bridge but I understand that is not correct in the design point of
> > > view.
> > >
> > > So this patch can not be queued because it's a known problem that
> > > people are discussing
> > >
> > Author: Michael Trimarchi
> > Date: Thu Dec 9 15:45:48 2021 +0100
> >
> > drm: bridge: samsung-dsim: Enable panel/bridge before exist from standby
> >
> > We need to exist from standby as last operation to have a proper video
> > working. This code implement the same code was before the bridge
> > migration
> >
> > Signed-off-by: Michael Trimarchi
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index 654851edbd9b..21265ae80022 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -1838,6 +1838,7 @@ static void samsung_dsim_atomic_enable(struct
> > drm_bridge *bridge,
> >struct drm_bridge_state
> > *old_bridge_state)
> > {
> > struct samsung_dsim *dsi = bridge_to_dsi(bridge);
> > + struct drm_atomic_state old_state;
> > int ret;
> >
> > if (dsi->state & DSIM_STATE_ENABLED)
> &
Re: [PATCH 2/2] drm/tiny: add support for tft displays based on ilitek, ili9488
d
> +* after unplug.
> +*/
> +
> + DRM_DEBUG_KMS("\n");
> +
> + mipi_dbi_command(>dbi, MIPI_DCS_SET_DISPLAY_OFF);
> +}
> +
> +static const u32 ili9488_formats[] = {
> + DRM_FORMAT_RGB565,
> +};
> +
> +static const struct drm_simple_display_pipe_funcs ili9488_pipe_funcs = {
> + .enable = ili9488_pipe_enable,
> + .disable = ili9488_pipe_disable,
> + .update = ili9488_pipe_update,
> +};
> +
> +static const struct drm_display_mode ili9488_mode = {
> + DRM_SIMPLE_MODE(320, 480, 49, 73),
> +};
> +
> +DEFINE_DRM_GEM_CMA_FOPS(ili9488_fops);
> +
> +static struct drm_driver ili9488_driver = {
> + .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> + .fops = _fops,
> + DRM_GEM_CMA_DRIVER_OPS_VMAP,
> + .debugfs_init = mipi_dbi_debugfs_init,
> + .name = "ili9488",
> + .desc = "Ilitek ILI9488",
> + .date = "20221017",
> + .major = 1,
> + .minor = 0,
> +};
> +
> +static const struct of_device_id ili9488_of_match[] = {
> + { .compatible = "waveshare,pico-rt-lcd-35" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ili9488_of_match);
> +
> +static const struct spi_device_id ili9488_id[] = {
> + { "ili9488", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(spi, ili9488_id);
> +
> +static int ili9488_probe(struct spi_device *spi)
> +{
> + struct device *dev = >dev;
> + struct mipi_dbi_dev *dbidev;
> + struct drm_device *drm;
> + struct mipi_dbi *dbi;
> + struct gpio_desc *dc;
> + u32 rotation = 0;
> + size_t bufsize;
> + int ret;
> +
> + dbidev = devm_drm_dev_alloc(dev, _driver,
> + struct mipi_dbi_dev, drm);
> + if (IS_ERR(dbidev))
> + return PTR_ERR(dbidev);
> +
> + dbi = >dbi;
> + drm = >drm;
> +
> + bufsize = ili9488_mode.vdisplay * ili9488_mode.hdisplay * 3;
> +
> + dbi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(dbi->reset)) {
> + DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
> + return PTR_ERR(dbi->reset);
> + }
> +
> + dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
> + if (IS_ERR(dc)) {
> + DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
> + return PTR_ERR(dc);
> + }
> +
> + dbidev->backlight = devm_of_find_backlight(dev);
> + if (IS_ERR(dbidev->backlight))
> + return PTR_ERR(dbidev->backlight);
> +
> + device_property_read_u32(dev, "rotation", );
> +
> + ret = mipi_dbi_spi_init(spi, dbi, dc);
> + if (ret)
> + return ret;
> +
> + dbidev->drm.mode_config.preferred_depth = 16;
> +
> + ret = mipi_dbi_dev_init_with_formats(dbidev, _pipe_funcs,
> +ili9488_formats,
> +ARRAY_SIZE(ili9488_formats),
> +_mode, rotation,
> bufsize);
> + if (ret)
> + return ret;
> +
> + dbi->swap_bytes = true;
> + drm_mode_config_reset(drm);
> +
> + ret = drm_dev_register(drm, 0);
> + if (ret)
> + return ret;
> +
> + spi_set_drvdata(spi, drm);
> +
> + drm_fbdev_generic_setup(drm, 0);
> +
> + return 0;
> +}
> +
> +static void ili9488_remove(struct spi_device *spi)
> +{
> + struct drm_device *drm = spi_get_drvdata(spi);
> +
> + drm_dev_unplug(drm);
> + drm_atomic_helper_shutdown(drm);
> +}
> +
> +static void ili9488_shutdown(struct spi_device *spi)
> +{
> + drm_atomic_helper_shutdown(spi_get_drvdata(spi));
> +}
> +
> +static struct spi_driver ili9488_spi_driver = {
> + .driver = {
> + .name = "ili9488",
> + .owner = THIS_MODULE,
> + .of_match_table = ili9488_of_match,
> + },
> + .id_table = ili9488_id,
> + .probe = ili9488_probe,
> + .remove = ili9488_remove,
> + .shutdown = ili9488_shutdown,
> +};
> +module_spi_driver(ili9488_spi_driver);
> +
> +MODULE_DESCRIPTION("Ilitek ILI9488 DRM driver");
> +MODULE_AUTHOR("Kamlesh Gurudasani ");
> +MODULE_AUTHOR("Michael Trimarchi ");
> +MODULE_AUTHOR("Tommaso Merciai ");
> +MODULE_LICENSE("GPL");
> \ No newline at end of file
> --
> 2.25.1
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH 0/2] drm/tiny: add support tft display based on ilitek, ili9488
Hi On Tue, Oct 18, 2022 at 9:06 PM Noralf Trønnes wrote: > > > > Den 18.10.2022 18.45, skrev Tommaso Merciai: > > Hi All, > > This series support for ilitek,ili9488 based displays like > > Waveshare-ResTouch-LCD-3.5 display. Tested on Waveshare-ResTouch-LCD-3.5 > > connected to px30-evb via SPI. > > There's a generic MIPI DBI SPI driver now that should work with all > these panels: drivers/gpu/drm/tiny/panel-mipi-dbi.c > > More info: https://github.com/notro/panel-mipi-dbi/wiki > We have seen it but it does not apply to the color output and there is no helper. I was a bit surprised to have a generic panel for spi and not for standard mipi one. Michael > Noralf. > > > This series is based on work done by Kamlesh Gurudasani in 2020: > > > > - "drm/tiny: add support for tft displays based on ilitek, ili9488" > > > > (Thanks Kamlesh for your starting point) > > > > Tests are done using the following tools coming from Yocto fs: > > > > - modetest -M "ili9488" -s 31:320x480@RG16 -v > > - fb-test > > - fb-rect > > > > References: > > - > > https://patchwork.kernel.org/project/dri-devel/patch/00719f68aca488a6476b0dda634617606b592823.1592055494.git.kamlesh.gurudas...@gmail.com/ > > - https://www.hpinfotech.ro/ILI9488.pdf > > - https://www.waveshare.com/wiki/Pico-ResTouch-LCD-3.5 > > > > Regards, > > Tommaso > > > > Tommaso Merciai (2): > > dt-bindings: add binding for tft displays based on ilitek,ili9488 > > drm/tiny: add support for tft displays based on ilitek,ili9488 > > > > .../bindings/display/ilitek,ili9488.yaml | 72 +++ > > drivers/gpu/drm/tiny/Kconfig | 13 + > > drivers/gpu/drm/tiny/Makefile | 1 + > > drivers/gpu/drm/tiny/ili9488.c | 440 ++ > > 4 files changed, 526 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/ilitek,ili9488.yaml > > create mode 100644 drivers/gpu/drm/tiny/ili9488.c > > -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com
Re: [RFC PATCH 0/4] Add RGB ttl connection on rockchip phy
Hi all
On Sun, Oct 2, 2022 at 8:45 AM Michael Trimarchi
wrote:
>
> The rockchip phy can be convigured in ttl mode. The phy is shared
> between lvds, dsi, ttl. The configuration that now I'm able to support
> has the display output on some set of pins on standard vop output
> and a set of pins using the ttl phy. The solution is not clean as I
> would like to have because some register that are used to enable
> the TTL, are in the same register area of the dsi controller.
> In order to test I must add the following
>
> dsi_dphy: phy@ff2e {
>
> reg = <0x0 0xff2e 0x0 0x1>,
> <0x0 0xff45 0x0 0x1>;
> ...
> }
>
> The problem here is the second region I have added is the same of
> dsi logic. Only one register is needed by the the phy driver
>
Is there anyone who has time to review it?
Michael
> Michael Trimarchi (4):
> phy: add PHY_MODE_TTL
> phy: rockchip: Add inno_is_valid_phy_mode
> phy: rockchip: Implement TTY phy mode
> drm/rockchip: rgb: Add dphy connection to rgb output
>
> drivers/gpu/drm/rockchip/rockchip_rgb.c | 18 +
> .../phy/rockchip/phy-rockchip-inno-dsidphy.c | 72 +++
> include/linux/phy/phy.h | 3 +-
> 3 files changed, 92 insertions(+), 1 deletion(-)
>
> --
> 2.34.1
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
display band (display area vs real visible area)
Hi all I would like to know the best approach in the graphics subsystem how deal with panels where the display area is different from the visible area because the display has a band left and right. I have already done the drm driver for the panel but from userspace point of view it's a pain to deal in wayland for input device and output device. Do you have any suggestions? Regards Michael
Re: display band (display area vs real visible area)
Hi On Tue, Mar 21, 2023 at 11:43 AM Jani Nikula wrote: > > On Tue, 21 Mar 2023, Michael Nazzareno Trimarchi > wrote: > > Hi all > > > > I would like to know the best approach in the graphics subsystem how > > deal with panels where the display area is different from the visible > > area because the display has a band left and right. I have already > > done the drm driver for the panel but from userspace point of view > > it's a pain to deal in wayland for input device and output device. Do > > you have any suggestions? > > Do you have the EDID for the panel? mipi->panel so should not have edid Michael > > BR, > Jani. > > -- > Jani Nikula, Intel Open Source Graphics Center -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com
Re: display band (display area vs real visible area)
Hi Daniel On Tue, Mar 21, 2023 at 1:15 PM Daniel Stone wrote: > > Hi, > > On Tue, 21 Mar 2023 at 12:08, Jani Nikula wrote: > > On Tue, 21 Mar 2023, Daniel Stone wrote: > > > There have been some threads - mostly motivated by MacBooks and the > > > Asahi team - about creating a KMS property to express invisible areas. > > > This would be the same thing, and the userspace ecosystem will pick it > > > up when the kernel exposes it. > > > > In my case the kernel handled it completely internally, and the > > userspace didn't even know. But I guess it depends on the display being > > able to take a smaller framebuffer, otherwise I don't think it's > > feasible. > > > > I haven't checked the threads you mention but I assume it covers the > > more general case of having rounded corners or holes for the camera, not > > just the frame covering the edges or something like that. That couldn't > > possibly be handled by kernel alone, but it's also a bunch of > > infrastructure work both in kernel and userspace to make it happen. > > Yeah, exactly. Just a connector property, which could come from DT or > ACPI or manual overrides or whatever. Userspace would still allocate a > full-size framebuffer, but look at that property and not render > anything useful into those areas. In the camera/notch case, it's a > matter of not putting useful content there. In the letterbox/pillarbox > case, it's about shrinking the reported screen size so that window > management, clients, etc, all believe that the screen is smaller than > it is. So it's up to wayland or compositor to take account of the side band, including touch controller. Am I right? Michael > > Cheers, > Daniel
Re: display band (display area vs real visible area)
Hi Daniel On Tue, Mar 21, 2023 at 12:49 PM Daniel Stone wrote: > > Hi, > > On Tue, 21 Mar 2023 at 11:24, Jani Nikula wrote: > > On Tue, 21 Mar 2023, Michael Nazzareno Trimarchi > > wrote: > > > On Tue, Mar 21, 2023 at 11:43 AM Jani Nikula > > > wrote: > > >> On Tue, 21 Mar 2023, Michael Nazzareno Trimarchi > > >> wrote: > > >> > I would like to know the best approach in the graphics subsystem how > > >> > deal with panels where the display area is different from the visible > > >> > area because the display has a band left and right. I have already > > >> > done the drm driver for the panel but from userspace point of view > > >> > it's a pain to deal in wayland for input device and output device. Do > > >> > you have any suggestions? > > >> > > >> Do you have the EDID for the panel? > > > > > > mipi->panel so should not have edid > > > > That's the kind of information you'd expect in the original question. ;) > > > > I've done that sort of thing in the past, but not sure if it would fly > > upstream. Basically the kernel driver would lie about the resolution to > > userspace, and handle the centering and the bands internally. In my > > case, the DSI command mode panel in question had commands to set the > > visible area, so the driver didn't have to do all that much extra to > > make it happen. > > There have been some threads - mostly motivated by MacBooks and the > Asahi team - about creating a KMS property to express invisible areas. > This would be the same thing, and the userspace ecosystem will pick it > up when the kernel exposes it. > > Cheers, > Daniel Any thread or patches? Michael
Re: [PATCH v3 4/4] drm/stm: add an option to change FB bpp
Hi
On Tue, Jun 13, 2023 at 4:41 PM Philippe CORNU
wrote:
>
>
>
> On 6/9/23 08:20, Dario Binacchi wrote:
> > Boards that use the STM32F{4,7} series have limited amounts of RAM. The
> > added parameter allows users to size, within certain limits, the memory
> > footprint required by the framebuffer.
> >
> > Signed-off-by: Dario Binacchi
> >
> > ---
> >
> > Changes in v3:
> > - drop [4/6] dt-bindings: display: simple: add Rocktech RK043FN48H
> >Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git
> > (drm-misc-next):
> >
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=c42a37a27c777d63961dd634a30f7c887949491a
> > - drop [5/6] drm/panel: simple: add support for Rocktech RK043FN48H panel
> >Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git
> > (drm-misc-next)
> >
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=13cdd12a9f934158f4ec817cf048fcb4384aa9dc
> >
> > drivers/gpu/drm/stm/drv.c | 8 +++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> > index 40df7d8c..65be2b442a6a 100644
> > --- a/drivers/gpu/drm/stm/drv.c
> > +++ b/drivers/gpu/drm/stm/drv.c
> > @@ -30,6 +30,11 @@
> > #define STM_MAX_FB_WIDTH2048
> > #define STM_MAX_FB_HEIGHT 2048 /* same as width to handle orientation */
> >
> > +static uint stm_bpp = 16;
> > +
> > +MODULE_PARM_DESC(bpp, "bits-per-pixel (default: 16)");
> > +module_param_named(bpp, stm_bpp, uint, 0644);
> > +
> > static const struct drm_mode_config_funcs drv_mode_config_funcs = {
> > .fb_create = drm_gem_fb_create,
> > .atomic_check = drm_atomic_helper_check,
> > @@ -93,6 +98,7 @@ static int drv_load(struct drm_device *ddev)
> > ddev->mode_config.min_height = 0;
> > ddev->mode_config.max_width = STM_MAX_FB_WIDTH;
> > ddev->mode_config.max_height = STM_MAX_FB_HEIGHT;
> > + ddev->mode_config.preferred_depth = stm_bpp;
> > ddev->mode_config.funcs = _mode_config_funcs;
> > ddev->mode_config.normalize_zpos = true;
> >
> > @@ -203,7 +209,7 @@ static int stm_drm_platform_probe(struct
> > platform_device *pdev)
> > if (ret)
> > goto err_put;
> >
> > - drm_fbdev_dma_setup(ddev, 16);
> > + drm_fbdev_dma_setup(ddev, stm_bpp);
> >
> > return 0;
> >
>
> Acked-by: Philippe Cornu
> Many thanks,
> Philippe :-)
>
According to the latest review on usb patchset: "Please do not add new
module parameters, this is not the 1990's anymore.
We have per-device settings everywhere, this makes that impossible.
Just use a DT value, if it is wrong, then fix the DT value! No need to
have the kernel override it, that's not what DT files are for."
I think it makes more sense to have dts parameters. Should maybe apply here too
Michael
Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
Hi Jagan On Wed, Dec 6, 2023 at 2:31 PM Jagan Teki wrote: > > Hi Dario, > > On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi > wrote: > > > > Hi Dave and Jagan, > > > > On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson > > wrote: > > > > > > Hi Dario > > > > > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi > > > wrote: > > > > > > > > The patch fixes the code for finding the next bridge with the > > > > "pre_enable_prev_first" flag set to false. In case this condition is > > > > not verified, i. e. there is no subsequent bridge with the flag set to > > > > false, the whole bridge list is traversed, invalidating the "next" > > > > variable. > > > > > > > > The use of a new iteration variable (i. e. "iter") ensures that the > > > > value > > > > of the "next" variable is not invalidated. > > > > > > We already have https://patchwork.freedesktop.org/patch/529288/ that > > > has been reviewed (but not applied) to resolve this. What does this > > > version do differently and why? > > > > My patches only affect drm_atomic_bridge_chain_post_disable(), whereas > > Jagan's patch affects both > > drm_atomic_bridge_chain_post_disable() and > > drm_atomic_bridge_chain_pre_enable(). > > I tested Jagan's patch on my system with success and I reviewed with > > Michael Trimarchi the > > drm_atomic_bridge_chain_pre_enable() fixing and we think it's okay. > > We also believe that our changes to post_disable() are better, as we > > set the 'next' variable only when required, > > and the code is more optimized since the list_is_last() is not called > > within the loop. > > Would it be possible to use Jagan's patch for fixing > > drm_atomic_bridge_chain_pre_enable() and mine for > > fixing drm_atomic_bridge_chain_post_disable()? > > > > Can you please share the post-disabled bridge chain list with the > below example before and after your change? We have already git commit the description in how the patch affects the post_disable. As Dario reported your patch is ok even in our use case. We don't have a real use case as the one you describe. Can we know how you test it in this use case here? Can you test our patches of post_disable? Thanks Michael > > Example: > - Panel > - Bridge 1 > - Bridge 2 pre_enable_prev_first > - Bridge 3 > - Bridge 4 pre_enable_prev_first > - Bridge 5 pre_enable_prev_first > - Bridge 6 > - Encoder > > Thanks, > Jagan.
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
gt; > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
> > > > > > Encoder.
> > > > > >
> > > > > > [post_disable]
> > > > > >
> > > > > > The altered bridge ordering has failed if two consecutive bridges
> > > > > > on a
> > > > > > given pipeline enables the pre_enable_prev_first flag.
> > > > > >
> > > > > > Example:
> > > > > > - Panel
> > > > > > - Bridge 1
> > > > > > - Bridge 2 pre_enable_prev_first
> > > > > > - Bridge 3
> > > > > > - Bridge 4 pre_enable_prev_first
> > > > > > - Bridge 5 pre_enable_prev_first
> > > > > > - Bridge 6
> > > > > > - Encoder
> > > > > >
> > > > > > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
> > > > > >
> > > > > > The logic looks for a bridge which enabled pre_enable_prev_first
> > > > > > flags
> > > > > > on each iteration and assigned the previou bridge to next and next
> > > > > > to
> > > > > > limit pointer if the bridge does enable pre_enable_prev_first flag.
> > > > > >
> > > > > > If control starts from Bridge 6 then it found next Bridge 5 is
> > > > > > pre_enable_prev_first and immediately the next assigned to previous
> > > > > > Bridge 6 and limit assignments to next Bridge 6 and call post_enable
> > > > > > of Bridge 6 even though the next consecutive Bridge 5 is enabled
> > > > > > with
> > > > > > pre_enable_prev_first. This clearly misses the logic to find the
> > > > > > state
> > > > > > of next conducive bridge as everytime the next and limit assigns
> > > > > > previous bridge if given bridge enabled pre_enable_prev_first.
> > > > > >
> > > > > > So, the resulting post_disable bridge order would be,
> > > > > > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge
> > > > > > 1,
> > > > > > Panel.
> > > > > >
> > > > > > This patch fixes this by assigning next with previou bridge only if
> > > > > > the
> > > > > > bridge doesn't enable pre_enable_prev_first flag and the next
> > > > > > further
> > > > > > assign it to limit. This way we can find the bridge that NOT
> > > > > > requested
> > > > > > prev to disable last.
> > > > > >
> > > > > > So, the resulting pre_enable bridge order with fix would be,
> > > > > > - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge
> > > > > > 1,
> > > > > > Panel.
> > > > > >
> > > > > > Validated the bridge init ordering by incorporating dummy bridges in
> > > > > > the sun6i-mipi-dsi pipeline
> > > > > >
> > > > > > Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
> > > > > > alter bridge init order")
> > > > > > Signed-off-by: Jagan Teki
> > > >
> > > > Thanks for investigating and sorting this.
> > > >
> > > > Reviewed-by: Dave Stevenson
> > > >
> > > > > > ---
> > > > > > Changes for v2:
> > > > > > - add missing dri-devel in CC
> > > > >
> > > > > Would you please look into this issue?
> > >
> > > These still not been picked it yet, can any one pull these two fixes?
> >
> > Ping!
>
Tested-by: Michael Trimarchi
> Ping!
>
> Thanks,
> Jagan.
>
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
Hi Jagan On Sun, Nov 26, 2023 at 5:11 PM Jagan Teki wrote: > > On Mon, Nov 13, 2023 at 6:45 PM Jagan Teki wrote: > > > > On Tue, Aug 1, 2023 at 11:50 AM Dave Stevenson > > wrote: > > > > > > Hi Jagan > > > > > > My apologies for dropping the ball on this one, and thanks to Frieder > > > for the nudge. > > > > > > On Wed, 12 Apr 2023 at 07:25, Jagan Teki > > > wrote: > > > > > > > > Hi Dave, > > > > > > > > Added Maxime, Laurent [which I thought I added before] > > > > > > > > On Tue, Mar 28, 2023 at 10:38 PM Jagan Teki > > > > wrote: > > > > > > > > > > For a given bridge pipeline if any bridge sets pre_enable_prev_first > > > > > flag then the pre_enable for the previous bridge will be called before > > > > > pre_enable of this bridge and opposite is done for post_disable. > > > > > > > > > > These are the potential bridge flags to alter bridge init order in > > > > > order > > > > > to satisfy the MIPI DSI host and downstream panel or bridge to > > > > > function. > > > > > However the existing pre_enable_prev_first logic with associated > > > > > bridge > > > > > ordering has broken for both pre_enable and post_disable calls. > > > > > > > > > > [pre_enable] > > > > > > > > > > The altered bridge ordering has failed if two consecutive bridges on a > > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > > > Example: > > > > > - Panel > > > > > - Bridge 1 > > > > > - Bridge 2 pre_enable_prev_first > > > > > - Bridge 3 > > > > > - Bridge 4 pre_enable_prev_first > > > > > - Bridge 5 pre_enable_prev_first > > > > > - Bridge 6 > > > > > - Encoder > > > > > > > > > > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first. > > > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first flag > > > > > on each iteration and assigned the previou bridge to limit pointer > > > > > if the bridge doesn't enable pre_enable_prev_first flags. > > > > > > > > > > If control found Bridge 2 is pre_enable_prev_first then the iteration > > > > > looks for Bridge 3 and found it is not pre_enable_prev_first and > > > > > assigns > > > > > it's previous Bridge 4 to limit pointer and calls pre_enable of > > > > > Bridge 3 > > > > > and Bridge 2 and assign iter pointer with limit which is Bridge 4. > > > > > > > > > > Here is the actual problem, for the next iteration control look for > > > > > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration > > > > > moved to Bridge 4 so this iteration skips the Bridge 4. The iteration > > > > > found Bridge 6 doesn't pre_enable_prev_first flags so the limit > > > > > assigned > > > > > to Encoder. From next iteration Encoder skips as it is the last bridge > > > > > for reverse order pipeline. > > > > > > > > > > So, the resulting pre_enable bridge order would be, > > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5. > > > > > > > > > > This patch fixes this by assigning limit to next pointer instead of > > > > > previous bridge since the iteration always looks for bridge that does > > > > > NOT request prev so assigning next makes sure the last bridge on a > > > > > given iteration what exactly the limit bridge is. > > > > > > > > > > So, the resulting pre_enable bridge order with fix would be, > > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4, > > > > > Encoder. > > > > > > > > > > [post_disable] > > > > > > > > > > The altered bridge ordering has failed if two consecutive bridges on a > > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > > > Example: > > > > > - Panel > > > > > - Bridge 1 > > > > > - Bridge 2 pre_enable_prev_first > > > > > - Bridge 3 > > > > > - Bridge 4 pre_enable_prev_first > > > > > - Bridge 5 pre_enable_prev_first > > > > > - Bridge 6 > > > > > - Encoder > > > > > > > > > > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first. > > > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first flags > > > > > on each iteration and assigned the previou bridge to next and next to > > > > > limit pointer if the bridge does enable pre_enable_prev_first flag. > > > > > > > > > > If control starts from Bridge 6 then it found next Bridge 5 is > > > > > pre_enable_prev_first and immediately the next assigned to previous > > > > > Bridge 6 and limit assignments to next Bridge 6 and call post_enable > > > > > of Bridge 6 even though the next consecutive Bridge 5 is enabled with > > > > > pre_enable_prev_first. This clearly misses the logic to find the state > > > > > of next conducive bridge as everytime the next and limit assigns > > > > > previous bridge if given bridge enabled pre_enable_prev_first. > > > > > > > > > > So, the resulting post_disable bridge order would be, > > > > > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1, > > > > > Panel. > > > > > > > > > > This patch fixes this by assigning next with previou bridge only if > > > > > the > > > > > bridge
Re: [PATCH v5 04/10] drm: bridge: samsung-dsim: complete the CLKLANE_STOP setting
Hi Frieder
On Thu, Dec 7, 2023 at 5:58 PM Frieder Schrempf
wrote:
>
> On 07.12.23 15:16, Dario Binacchi wrote:
> > The patch completes the setting of CLKLANE_STOP for the imx8mn and imx8mp
> > platforms (i. e. not exynos).
>
> This also affects i.MX8MM, so better just mention i.MX in general in the
> commit message.
>
> >
> > Co-developed-by: Michael Trimarchi
> > Signed-off-by: Michael Trimarchi
> > Signed-off-by: Dario Binacchi
> > ---
> >
> > (no changes since v1)
> >
> > drivers/gpu/drm/bridge/samsung-dsim.c | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> > b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index 15bf05b2bbe4..13f181c99d7e 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -96,6 +96,7 @@
> > #define DSIM_MFLUSH_VS BIT(29)
> > /* This flag is valid only for exynos3250/3472/5260/5430 */
> > #define DSIM_CLKLANE_STOPBIT(30)
> > +#define DSIM_NON_CONTINUOUS_CLKLANE BIT(31)
> >
> > /* DSIM_ESCMODE */
> > #define DSIM_TX_TRIGGER_RST BIT(4)
> > @@ -945,8 +946,12 @@ static int samsung_dsim_init_link(struct samsung_dsim
> > *dsi)
> >* power consumption.
> >*/
> > if (driver_data->has_clklane_stop &&
> > - dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)
> > + dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
> > + if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
> > + reg |= DSIM_NON_CONTINUOUS_CLKLANE;
> > +
> > reg |= DSIM_CLKLANE_STOP;
> > + }
>
> I really wonder what the difference between DSIM_NON_CONTINUOUS_CLKLANE
> and DSIM_CLKLANE_STOP is.
>
> If Exynos only has the latter, it's pretty clear what to use. But as
> i.MX has both of these bits, should both be set? Or is setting
> DSIM_NON_CONTINUOUS_CLKLANE enough and we should leave DSIM_CLKLANE_STOP
> alone?
>
We add the DSIM_NON_CONTINUOUS_CLKLANE because there was a similar
commit in NXP bsp.
Now according to the datasheet the DSIM_NON_CONTINUOUS_CLKLANE should
be the right bit.
NXP guys should clarify then a bit
Michael
> Maybe someone has a clue here. The description of the bits in the RM is:
>
> DSIM_NON_CONTINUOUS_CLKLANE - Non-continuous clock mode
> DSIM_CLKLANE_STOP - PHY clock lane On/Off for ESD
>
> > samsung_dsim_write(dsi, DSIM_CONFIG_REG, reg);
> >
> > lanes_mask = BIT(dsi->lanes) - 1;
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
Hi Robert On Tue, Mar 5, 2024 at 3:54 PM Robert Foss wrote: > > On Tue, 28 Mar 2023 22:37:51 +0530, Jagan Teki wrote: > > For a given bridge pipeline if any bridge sets pre_enable_prev_first > > flag then the pre_enable for the previous bridge will be called before > > pre_enable of this bridge and opposite is done for post_disable. > > > > These are the potential bridge flags to alter bridge init order in order > > to satisfy the MIPI DSI host and downstream panel or bridge to function. > > However the existing pre_enable_prev_first logic with associated bridge > > ordering has broken for both pre_enable and post_disable calls. > > > > [...] > > Please excuse the delay, patches touching the core bridge code are a little > bit tougher to merge due to increased risks of breaking unrelated things. > > Applied, thanks! > I have a question about this prev_first flag. Can we map the order in the connector in dts? Michael > [1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e18aeeda0b69 > [2/2] drm/bridge: Document bridge init order with pre_enable_prev_first > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=113cc3ad8566 > > > > Rob > > -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com
