Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230415104104.5537-1-aford173@gmail.com> <3e47f0d1017fe4c9f71a5de65f32c6ba1662efe2.camel@pengutronix.de>
In-Reply-To: <3e47f0d1017fe4c9f71a5de65f32c6ba1662efe2.camel@pengutronix.de>
From:   Adam Ford <aford173@gmail.com>
Date:   Mon, 17 Apr 2023 06:55:30 -0500
Message-ID: <CAHCN7xL4+9NogrnXA1PEWorwY7JpSGBozDtHT83JvzjfinmS+A@mail.gmail.com>
Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations
To:     Lucas Stach <l.stach@pengutronix.de>
Cc:     dri-devel@lists.freedesktop.org,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        aford@beaconembedded.com,
        Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
        Andrzej Hajda <andrzej.hajda@intel.com>,
        Fabio Estevam <festevam@gmail.com>, m.szyprowski@samsung.com,
        marex@denx.de, Robert Foss <rfoss@kernel.org>,
        David Airlie <airlied@gmail.com>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Jagan Teki <jagan@amarulasolutions.com>,
        NXP Linux Team <linux-imx@nxp.com>,
        devicetree@vger.kernel.org, Daniel Vetter <daniel@ffwll.ch>,
        Jonas Karlman <jonas@kwiboo.se>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Inki Dae <inki.dae@samsung.com>,
        Rob Herring <robh+dt@kernel.org>,
        linux-arm-kernel@lists.infradead.org,
        Neil Armstrong <neil.armstrong@linaro.org>,
        linux-kernel@vger.kernel.org,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Shawn Guo <shawnguo@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Apr 17, 2023 at 3:43=E2=80=AFAM Lucas Stach <l.stach@pengutronix.de=
> wrote:
>
> Hi Adam,
>
> Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> > If there is more than one lane, the HFP, HBP, and HSA is calculated in
> > bytes/pixel, then they are divided amongst the different lanes with som=
e
> > additional overhead. This is necessary to achieve higher resolutions wh=
ile
> > keeping the pixel clocks lower as the number of lanes increase.
> >
>
> In the testing I did to come up with my patch "drm: bridge: samsung-
> dsim: fix blanking packet size calculation" the number of lanes didn't
> make any difference. My testing might be flawed, as I could only
> measure the blanking after translation from MIPI DSI to DPI, so I'm
> interested to know what others did here. How did you validate the
> blanking with your patch? Would you have a chance to test my patch and
> see if it works or breaks in your setup?

Mine was purely by trial and error.  I don't have a scope, nor do I
have a copy of the MIPI DSI spec, so if the image sync'd with my
monitor, I treated it as successful.

I can give yours a try, but it might be a few days since I've only
been working on this stuff a bit in my spare time.

adam

>
> Regards,
> Lucas
>
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> > ---
> >  drivers/gpu/drm/bridge/samsung-dsim.c | 40 +++++++++++++++++++++++----
> >  1 file changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/br=
idge/samsung-dsim.c
> > index e0a402a85787..1ccbad4ea577 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -215,6 +215,7 @@
> >  #define DSI_RX_FIFO_SIZE             256
> >  #define DSI_XFER_TIMEOUT_MS          100
> >  #define DSI_RX_FIFO_EMPTY            0x30800002
> > +#define DSI_HSYNC_PKT_OVERHEAD       6
> >
> >  #define OLD_SCLK_MIPI_CLK_NAME               "pll_clk"
> >
> > @@ -879,13 +880,40 @@ static void samsung_dsim_set_display_mode(struct =
samsung_dsim *dsi)
> >                       | DSIM_MAIN_VBP(m->vtotal - m->vsync_end);
> >               samsung_dsim_write(dsi, DSIM_MVPORCH_REG, reg);
> >
> > -             reg =3D DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
> > -                     | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
> > -             samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> > +             /*
> > +              * If there is more than one lane, the HFP, HBP, and HSA
> > +              * is calculated in bytes/pixel, then they are divided
> > +              * amongst the different lanes with some additional
> > +              * overhead correction
> > +              */
> > +             if (dsi->lanes > 1) {
> > +                     u32 hfp, hbp, hsa;
> > +                     int bpp =3D mipi_dsi_pixel_format_to_bpp(dsi->for=
mat) / 8;
> > +
> > +                     hfp =3D ((m->hsync_start - m->hdisplay) * bpp) / =
dsi->lanes;
> > +                     hfp -=3D (hfp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSY=
NC_PKT_OVERHEAD : 0;
> > +
> > +                     hbp =3D ((m->htotal - m->hsync_end) * bpp) / dsi-=
>lanes;
> > +                     hbp -=3D (hbp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSY=
NC_PKT_OVERHEAD : 0;
> >
> > -             reg =3D DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> > -                     | DSIM_MAIN_HSA(m->hsync_end - m->hsync_start);
> > -             samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> > +                     hsa =3D ((m->hsync_end - m->hsync_start) * bpp) /=
 dsi->lanes;
> > +                     hsa -=3D (hsa > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSY=
NC_PKT_OVERHEAD : 0;
> > +
> > +                     reg =3D DSIM_MAIN_HFP(hfp) | DSIM_MAIN_HBP(hbp);
> > +                     samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> > +
> > +                     reg =3D DSIM_MAIN_VSA(m->vsync_end - m->vsync_sta=
rt)
> > +                             | DSIM_MAIN_HSA(hsa);
> > +                     samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> > +             } else {
> > +                     reg =3D DSIM_MAIN_HFP(m->hsync_start - m->hdispla=
y)
> > +                             | DSIM_MAIN_HBP(m->htotal - m->hsync_end)=
;
> > +                     samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> > +
> > +                     reg =3D DSIM_MAIN_VSA(m->vsync_end - m->vsync_sta=
rt)
> > +                             | DSIM_MAIN_HSA(m->hsync_end - m->hsync_s=
tart);
> > +                     samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> > +             }
> >       }
> >       reg =3D  DSIM_MAIN_HRESOL(m->hdisplay, num_bits_resol) |
> >               DSIM_MAIN_VRESOL(m->vdisplay, num_bits_resol);
>