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: <20230423121232.1345909-1-aford173@gmail.com> <CGME20230423121305eucas1p287a952d41b1884b117fa15a748b9e1a2@eucas1p2.samsung.com>
 <20230423121232.1345909-6-aford173@gmail.com> <b6b53da5-6986-a958-ef84-650b3a57ad9c@samsung.com>
 <CAHCN7x+vd-bP8NgS-cRrnm8ojq0kwUg6aXokJv6xSU7BrT04Vw@mail.gmail.com>
 <343f8d25-566f-9d14-64db-4e796cc9e406@samsung.com> <CAHCN7x+QU29Wv9TQEANhbxLcL4jCZUKbk+uGu2sOwhCcTt798A@mail.gmail.com>
 <46429c9b-cf14-a67e-81a8-b56be0350ea3@samsung.com>
In-Reply-To: <46429c9b-cf14-a67e-81a8-b56be0350ea3@samsung.com>
From:   Adam Ford <aford173@gmail.com>
Date:   Fri, 28 Apr 2023 09:09:44 -0500
Message-ID: <CAHCN7xKcHELG3n8+f4pjj05NCFUhBfz0_q_huN9bP65Fv6Ombg@mail.gmail.com>
Subject: Re: [PATCH V2 5/6] drm: bridge: samsung-dsim: Support non-burst mode
To:     Marek Szyprowski <m.szyprowski@samsung.com>
Cc:     dri-devel@lists.freedesktop.org, aford@beaconembedded.com,
        dario.binacchi@amarulasolutions.com, l.stach@pengutronix.de,
        Inki Dae <inki.dae@samsung.com>,
        Jagan Teki <jagan@amarulasolutions.com>,
        Andrzej Hajda <andrzej.hajda@intel.com>,
        Neil Armstrong <neil.armstrong@linaro.org>,
        Robert Foss <rfoss@kernel.org>,
        Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
        Jonas Karlman <jonas@kwiboo.se>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>, Marek Vasut <marex@denx.de>,
        Frieder Schrempf <frieder.schrempf@kontron.de>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Apr 28, 2023 at 9:04=E2=80=AFAM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> On 28.04.2023 15:35, Adam Ford wrote:
> > On Fri, Apr 28, 2023 at 7:31=E2=80=AFAM Marek Szyprowski
> > <m.szyprowski@samsung.com> wrote:
> >> On 24.04.2023 12:00, Adam Ford wrote:
> >>> On Mon, Apr 24, 2023 at 3:25=E2=80=AFAM Marek Szyprowski
> >>> <m.szyprowski@samsung.com> wrote:
> >>>> On 23.04.2023 14:12, Adam Ford wrote:
> >>>>> The high-speed clock is hard-coded to the burst-clock
> >>>>> frequency specified in the device tree.  However, when
> >>>>> using devices like certain bridge chips without burst mode
> >>>>> and varying resolutions and refresh rates, it may be
> >>>>> necessary to set the high-speed clock dynamically based
> >>>>> on the desired pixel clock for the connected device.
> >>>>>
> >>>>> This also removes the need to set a clock speed from
> >>>>> the device tree for non-burst mode operation, since the
> >>>>> pixel clock rate is the rate requested from the attached
> >>>>> device like an HDMI bridge chip.  This should have no
> >>>>> impact for people using burst-mode and setting the burst
> >>>>> clock rate is still required for those users.
> >>>>>
> >>>>> Signed-off-by: Adam Ford <aford173@gmail.com>
> >>>> This one breaks Exynos-5433 based TM2e board with a DSI panel.
> >>> Marek S,
> >>>
> >>> Thank you for testing!  I knoiw there are several of us who appreciat=
e
> >>> your testing this since it's hard to know if something broke without
> >>> hardware.  Is there any way you can tell me if the flag is set to
> >>> enable MIPI_DSI_MODE_VIDEO_BURST?
> >> TM2e board uses the DSI panel operated in command mode and handled by
> >> panel-samsung-s6e3ha2.c driver. The MIPI_DSI_MODE_VIDEO_BURST flag is
> >> not set by the driver. However, the MIPI_DSI_CLOCK_NON_CONTINUOUS flag=
s
> >> is set there. I really have no idea if setting VIDEO_BURST would make
> >> sense together with CLOCK_NON_CONTINUOUS or not. Maybe the driver lack=
s
> >> setting it?
> >>
> >>
> >>> I was trying to be diligent about not breaking your boards, but
> >>> without your boards, it's difficult.  The theory was that if
> >>> MIPI_DSI_MODE_VIDEO_BURST is set and there is a burst clock set in th=
e
> >>> device tree, it would use the burst clock.
> >>>
> >>> As a fall-back I could just simply check for the presence of the
> >>> burst_clock_rate instead of both MIPI_DSI_MODE_VIDEO_BURST and
> >>> burst_clock_rate.
> >> Maybe you should extend your check also for the
> >> MIPI_DSI_CLOCK_NON_CONTINUOUS flag? Does it make sense?
> > Looking at some of the devices that might attach in the future, It
> > appears that ti-sn65dsi86.c sets this flag.  It's a display port
> > bridge, so I would expect it to need a variable clock rate similar to
> > how the HDMI bridge that I need works.  I am concerned that I make the
> > burst clock dependent on MIPI_DSI_CLOCK_NON_CONTINUOUS, it might break
> > the Display Port bridge.
> >
> > I think it's better to just check if the samsung,burst-clock-frequency
> > is present in the device tree and use it when present.  If it's not
> > present, then fall back to the pixel clock of the connected device.
>
> Right, this sounds rational.
>
> > I looked at a bunch of Exynos parts, and it looks like they all use
> > the samsung,burst-clock-frequency device tree setting.  Is that true,
> > or did I miss one?
>
> That true. All Exynos based boards with DSI panels use constant DSI
> burst frequency defined in the device tree.

Thanks for the feedback.  I'll try to get another rev of this series
pushed later today or Monday.

adam
>
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>