[PATCH 4/4] drm/msm/dp: Add DisplayPort controller for QCS8300

[Dmitry Baryshkov]

On Fri, Nov 29, 2024 at 06:15:56PM +0800, Yongxing Mou wrote:
> 
> 
> On 2024/11/27 21:43, Dmitry Baryshkov wrote:
> > On Wed, Nov 27, 2024 at 04:15:51PM +0800, Yongxing Mou wrote:
> > > The Qualcomm QCS8300 platform comes with a DisplayPort controller
> > > with same base offset with SA8775P, add support for this in the
> > > DisplayPort driver.
> > 
> > Can we reuse SA8775P config then? And SA8775p compatible as a fallback,
> > not requiring any driver modifications.
> > 
> for DP controller,in qcs8300, it only support controller_0,connection of
> controller_1 are removed. but sa8775p have two dpu,each of them support two
> controllers. So is it fine to reuse sa8775p?

Please take a look around in the file, you will notice the answer for
this question. Then take another look around, find the data that you
should have used instead, remove the useless mention of SA8775P and
mention the correct platform instead.

Or, better, drop the patch completely and modify the schema in a correct
way.

Please, always make a broader look around when writing the code, rather
than implementing a feature in a most obvious (but not always optimal)
way.

> > > 
> > > Signed-off-by: Yongxing Mou <quic_yongmou at quicinc.com>
> > > ---
> > >   drivers/gpu/drm/msm/dp/dp_display.c | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> > > index aba925aab7ad7c6652e81004043864c1cb3ac370..f870faa89f26a8cb5bd7f4caf11f42e919c9efac 100644
> > > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > > @@ -118,6 +118,11 @@ struct msm_dp_desc {
> > >   	bool wide_bus_supported;
> > >   };
> > > +static const struct msm_dp_desc msm_dp_desc_qcs8300[] = {
> > > +	{ .io_start = 0x0af54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },

I don't know what has happened here. I doublechecked, original patch
uses Tabs in the beginning of the lines, so it's okay.

> > > +	{}
> > > +};
> > > +
> > >   static const struct msm_dp_desc msm_dp_desc_sa8775p[] = {
> > >   	{ .io_start = 0x0af54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true },
> > >   	{ .io_start = 0x0af5c000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true },
> > > @@ -170,6 +175,7 @@ static const struct msm_dp_desc msm_dp_desc_x1e80100[] = {
> > >   };
> > >   static const struct of_device_id msm_dp_dt_match[] = {
> > > +	{ .compatible = "qcom,qcs8300-dp", .data = &msm_dp_desc_qcs8300 },
> > >   	{ .compatible = "qcom,sa8775p-dp", .data = &msm_dp_desc_sa8775p },
> > >   	{ .compatible = "qcom,sc7180-dp", .data = &msm_dp_desc_sc7180 },
> > >   	{ .compatible = "qcom,sc7280-dp", .data = &msm_dp_desc_sc7280 },
> > > 
> > > -- 
> > > 2.34.1
> > > 
> > 
> 

-- 
With best wishes
Dmitry