Re: [RFC PATCH v2 4/5] drm/msm/dp: replace dp_connector with drm_bridge_connector

From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Stephen Boyd <swboyd@chromium.org>
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Kuogee Hsieh <quic_khsieh@quicinc.com>,
	Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org
Subject: Re: [RFC PATCH v2 4/5] drm/msm/dp: replace dp_connector with drm_bridge_connector
Date: Sat, 19 Feb 2022 05:22:46 +0300	[thread overview]
Message-ID: <CAA8EJpobtpc5mB48g6K=+KaZQ-o8m_QTZr-dQvwz-9cEwiJ_Kg@mail.gmail.com> (raw)
In-Reply-To: <CAE-0n51afuHURLHaZBa77H_n+cm4Tj1Du-rpLH-HsrkY5xQVJA@mail.gmail.com>

On Sat, 19 Feb 2022 at 03:55, Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Dmitry Baryshkov (2022-02-18 14:32:53)
> > On 19/02/2022 00:31, Kuogee Hsieh wrote:
> > >
> > > On 2/11/2022 2:40 PM, Dmitry Baryshkov wrote:
> > >> There is little point in having both connector and root bridge
> > >> implementation in the same driver. Move connector's functionality to the
> > >> bridge to let next bridge in chain to override it.
> > >>
> > >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >
> > > This patch break primary (edp) display
> > >
> > > -- right half of screen garbled
> > >
> > > -- screen shift vertically
> > >
> > > below are error messages seen --
> > >
> > > [   36.679216] panel-edp soc@0:edp_panel: No display modes
> > > [   36.687272] panel-edp soc@0:edp_panel: No display modes
> > > [   40.593709] panel-edp soc@0:edp_panel: No display modes
> > > [   40.600285] panel-edp soc@0:edp_panel: No display modes
> >
> > So, before the patch the drm core was getting modes from the
> > drm_connector (which means, modes from drm driver itself). With this
> > patch the panel-edp tries to get modes.
> >
> > Could you please check, why panel_edp_get_modes() fails? Assuming that
> > you use platform panel-edp binding (rather than 'edp-panel') could you
> > please check you have either of the following:
> > - ddc bus for EDID?
>
> I don't see anywhere where the ddc pointer is set for the dp bridge in
> msm_dp_bridge_init(). Is that required though? I'd think simple panel is
> still being used here so reading EDID isn't required.

I meant the 'ddc-i2c-bus' property for the corresponding eDP panel.

> > - either num_timing or num_modes in your panel desc.

After reading the panel-edp's code I don't have another cause for
panel_edp_get_modes(). It should either have a DDC bus specified using
the mentioned device tree property, or it should have specified the
timings.

Kuogee, which platform were you using when testing this patch? Could
you please share the dts fragment?

-- 
With best wishes
Dmitry