[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZAiPTat/kmLyaJmA@Gentoo>
Date: Wed, 8 Mar 2023 21:37:59 +0800
From: Jianhua Lu <lujianhua000@...il.com>
To: Konrad Dybcio <konrad.dybcio@...aro.org>
Cc: Thierry Reding <thierry.reding@...il.com>,
Sam Ravnborg <sam@...nborg.org>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Linus Walleij <linus.walleij@...aro.org>,
Neil Armstrong <neil.armstrong@...aro.org>,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] drm/panel: Add driver for Novatek NT36523
On Wed, Mar 08, 2023 at 01:33:39PM +0100, Konrad Dybcio wrote:
>
> phy-type:
> description: D-PHY (default) or C-PHY mode
> enum: [ 10, 11 ]
> default: 10
>
> try setting that to 11 under your DSI PHYs (or one? not sure).
I already used phy-type = <PHY_TYPE_CPHY> before, this equate to qcom,panel-cphy-mode in
downstream panel device tree, but there is qcom,mdss-dsi-cphy-strength isn't
implemented.
> Konrad
> >
> >> Could you also implement a 60 (or whatever other value is also
> >> implemented downstream) Hz mode?
> > I will try to implement it.
> >>
> >>> + .clock = (1600 + 60 + 8 + 60) * (2560 + 26 + 4 + 168) * 104 / 1000,
> >>> + .hdisplay = 1600,
> >>> + .hsync_start = 1600 + 60,
> >>> + .hsync_end = 1600 + 60 + 8,
> >>> + .htotal = 1600 + 60 + 8 + 60,
> >>> + .vdisplay = 2560,
> >>> + .vsync_start = 2560 + 26,
> >>> + .vsync_end = 2560 + 26 + 4,
> >>> + .vtotal = 2560 + 26 + 4 + 168,
> >>> +};
> >>> +
> >>> +static const struct drm_display_mode elish_csot_mode = {
> >>> + /* Current max freqency is 104HZ, TODO 120HZ */
> >>> + .clock = (1600 + 200 + 40 + 52) * (2560 + 26 + 4 + 168) * 104 / 1000,
> >>> + .hdisplay = 1600,
> >>> + .hsync_start = 1600 + 200,
> >>> + .hsync_end = 1600 + 200 + 40,
> >>> + .htotal = 1600 + 200 + 40 + 52,
> >>> + .vdisplay = 2560,
> >>> + .vsync_start = 2560 + 26,
> >>> + .vsync_end = 2560 + 26 + 4,
> >>> + .vtotal = 2560 + 26 + 4 + 168,
> >>> +};
> >>> +
> >>> +static const struct panel_desc elish_boe_desc = {
> >>> + .modes = &elish_boe_mode,
> >>> + .dsi_info = {
> >>> + .type = "NT36523",
> >> That's a bit vague, the driver IC is not very telling about the
> >> panel itself. Since we're not able to determine much more information,
> >> this could contain the panel manufacturer and the device name.
> > Acked.
> >>
> >>> + .channel = 0,
> >>> + .node = NULL,
> >>> + },
> >>> + .width_mm = 127,
> >>> + .height_mm = 203,
> >>> + .bpc = 8,
> >>> + .lanes = 3,
> >>> + .format = MIPI_DSI_FMT_RGB888,
> >>> + .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM,
> >>> + .init_sequence = elish_boe_init_sequence,
> >>> + .is_dual_dsi = true,
> >>> +};
> >>> +
> >> [...]
> >>
> >>> +static int nt36523_probe(struct mipi_dsi_device *dsi)
> >>> +{
> >> [...]
> >>
> >>> + /* If the panel is dual dsi, register DSI1 */
> >>> + if (pinfo->desc->is_dual_dsi) {
> >>> + info = &pinfo->desc->dsi_info;
> >>> +
> >>> + dsi1 = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
> >>> + if (!dsi1) {
> >>> + dev_err(dev, "cannot get secondary DSI node.\n");
> >>> + return -ENODEV;
> >>> + }
> >>> +
> >>> + dsi1_host = of_find_mipi_dsi_host_by_node(dsi1);
> >>> + of_node_put(dsi1);
> >> Shouldn't you put the reference only if it's found?
> > thanks for spot it.
> >>
> >>> + if (!dsi1_host) {
> >>> + dev_err(dev, "cannot get secondary DSI host\n");
> >>> + return -EPROBE_DEFER;
> >> dev_err_probe, here and in neighbouring exit return paths?
> > Acked.
> >>
> >>
> >> Konrad
Powered by blists - more mailing lists