Re: [PATCH v6 6/6] drm/sprd: add Unisoc's drm mipi dsi&dphy driver

[Kevin Tang]

Sam,
Thanks for your response.

Sam Ravnborg <sam@xxxxxxxxxxxx> 于2021年9月27日周一 上午12:33写道：
>
> Hi Kevin,
>
> > > > +     reg->_0b.bits.out_sel = pll->out_sel;
> > > > +     reg->_0b.bits.kint_l = pll->kint & 0xf;
> > > > +     reg->_0e.bits.pll_pu_byp = 0;
> > > > +     reg->_0e.bits.pll_pu = 0;
> > > > +     reg->_0e.bits.stopstate_sel = 1;
> > > > +     reg->_0f.bits.det_delay = pll->det_delay;
> > > > +
> > > > +     val = (u8 *)&reg;
> > > > +
> > > > +     for (i = 0; i < sizeof(reg_addr); ++i) {
> > > > +             regmap_write(regmap, reg_addr[i], val[i]);
> > > > +             DRM_DEBUG("%02x: %02x\n", reg_addr[i], val[i]);
> > > > +     }
> > > > +}
> > >
> > > It would be great to also convert this part to a pattern without
> > > structures.
> > I will try it, but our pll registers, all not have official name from
> > aisc design owner.
> > If need to convert it, our pll regiters macro define, it can only be
> > named as reg_01, reg_02...
> IT is better to use the stupid names provided by you asic design owner,
> than just numbers. Add a comment explaining this is due to the asic
> design owner the names are stupid and not your fault.
The chip manual released to us by the asic designer is like this for
many years ago.
As for the specific reason, unable to find out now, i will try use
"stupid names" rather than numbers.
>
> > > > +
> > > > +     if (dsi->panel) {
> > > > +             drm_panel_prepare(dsi->panel);
> > > > +             drm_panel_enable(dsi->panel);
> > > > +     }
> > >
> > > Please use the new devm_drm_of_get_bridge helper here, and use the
> > > bridge API instead.
> > We use drm_panel_init and drm_panel_add API to add panel, so here is a
> > panel device, not a bridge.
>
> The new way to do this is to always wrap the panel in a bridge. We will
> start to slowly migrate away from direct use of the panel API, and let
> display drivers always wrap the panles in a bridge.
> So please do as Maxime suggests.
I only found a demo about the use of devm_drm_of_get_bridge in the vc4
dsi driver, i
will try to follow it.
>
>         Sam