On Mon, Jun 17, 2024 at 1:17 AM Dominique MARTINET
<dominique.martinet@xxxxxxxxxxxxxxxxx> wrote:
>
> Adam Ford wrote on Sat, Feb 03, 2024 at 10:52:50AM -0600:
> > From: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> >
> > Add a simple wrapper driver for the DWC HDMI bridge driver that
> > implements the few bits that are necessary to abstract the i.MX8MP
> > SoC integration.
>
> Hi Lucas, Adam,
> (trimmed ccs a bit)
>
> First, thank you for the effort of upstreaming all of this!! It's really
> appreciated, and with display working I'll really be wanting to upstream
> our DTS as well as soon as I have time (which is going to be a while,
> but better late than never ?)
>
> Until then, it's been a few months but I've got a question on this bit:
>
> > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > new file mode 100644
> > index 000000000000..89fc432ac611
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
> > +static enum drm_mode_status
> > +imx8mp_hdmi_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
> > + const struct drm_display_info *info,
> > + const struct drm_display_mode *mode)
> > +{
> > + struct imx8mp_hdmi *hdmi = (struct imx8mp_hdmi *)data;
> > +
> > + if (mode->clock < 13500)
> > + return MODE_CLOCK_LOW;
> > +
> > + if (mode->clock > 297000)
> > + return MODE_CLOCK_HIGH;
> > +
> > + if (clk_round_rate(hdmi->pixclk, mode->clock * 1000) !=
> > + mode->clock * 1000)
> > + return MODE_CLOCK_RANGE;
>
> Do you know why such a check is here?
I didn't write the original code, so I'll defer to Lucas here. I just
tried to edit/fix issues as they were identified to get it pushed
upstream.
>
> When plugging in a screen with no frequency identically supported in its
> EDID this check causes the screen to stay black, and we've been telling
> customers to override the EDID but it's a huge pain.
>
> Commit 6ad082bee902 ("phy: freescale: add Samsung HDMI PHY") already
> "fixed" the samsung hdmi phy driver to return the next frequency if an
> exact match hasn't been found (NXP tree's match frequencies exactly, but
> this gets the first clock with pixclk <= rate), so if this check is also
> relaxed our displays would work out of the box.
Are you proposing to replace 'return MODE_CLOCK_RANGE' with a printed warning?
>
> I also don't see any other bridge doing this kind of check.
> drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c has a similar check with a
> 0.5% leeway, and all the other drivers don't check anything.
> If you want to add some level of safety, I think we could make this work
> with a 5% margin easily... Printing a warning in dmesg could work if
> you're worried about artifacts, but litteraly anything is better than a
> black screen with no error message in my opinion.
>
>
> In practice the screen I'm looking at has an EDID which only supports
> 51.2MHz and the closest frequency supported by the Samsung HDMI phy is
> 50.4MHz, so that's a ~1.5% difference and it'd be great if it could work
> out of the box.
I wonder if the HDMI PHY could be improved to better dynamically
calculate values instead of the look tables.
adam
>
> For reference, the output of edid-decode is as follow:
> ---
> edid-decode /sys/devices/platform/display-subsystem/drm/car
> d1/card1-HDMI-A-1/edid
> edid-decode (hex):
>
> 00 ff ff ff ff ff ff 00 3a 49 03 00 01 00 00 00
> 20 1e 01 03 80 10 09 00 0a 00 00 00 00 00 00 00
> 00 00 00 00 00 00 01 01 01 01 01 01 01 01 01 01
> 01 01 01 01 01 01 00 14 00 40 41 58 23 20 a0 20
> c8 00 9a 56 00 00 00 18 00 00 00 10 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 10 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 10
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 9a
>
> ----------------
>
> Block 0, Base EDID:
> EDID Structure Version & Revision: 1.3
> Vendor & Product Identification:
> Manufacturer: NRI
> Model: 3
> Serial Number: 1
> Made in: week 32 of 2020
> Basic Display Parameters & Features:
> Digital display
> Maximum image size: 16 cm x 9 cm
> Gamma: 1.00
> RGB color display
> First detailed timing is the preferred timing
> Color Characteristics:
> Red : 0.0000, 0.0000
> Green: 0.0000, 0.0000
> Blue : 0.0000, 0.0000
> White: 0.0000, 0.0000
> Established Timings I & II: none
> Standard Timings: none
> Detailed Timing Descriptors:
> DTD 1: 1024x600 59.993 Hz 128:75 38.095 kHz 51.200 MHz (154 mm x 86 m
> m)
> Hfront 160 Hsync 32 Hback 128 Hpol N
> Vfront 12 Vsync 8 Vback 15 Vpol N
> Dummy Descriptor:
> Dummy Descriptor:
> Dummy Descriptor:
> Checksum: 0x9a
> ---
>
>
> Thanks,
> --
> Dominique Martinet
>
>
>
> --
> linux-phy mailing list
> linux-phy@xxxxxxxxxxxxxxxxxxx
> https://lists.infradead.org/mailman/listinfo/linux-phy
