[PATCH v4 10/10] drm/vs: add simple dsi encoder

Keith Zhao keith.zhao at starfivetech.com
Tue Jun 25 08:33:48 UTC 2024



> -----Original Message-----
> From: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> Sent: 2024年6月24日 5:11
> To: Keith Zhao <keith.zhao at starfivetech.com>
> Cc: andrzej.hajda at intel.com; neil.armstrong at linaro.org; rfoss at kernel.org;
> Laurent.pinchart at ideasonboard.com; jonas at kwiboo.se;
> jernej.skrabec at gmail.com; maarten.lankhorst at linux.intel.com;
> mripard at kernel.org; tzimmermann at suse.de; airlied at gmail.com;
> daniel at ffwll.ch; robh at kernel.org; krzk+dt at kernel.org; conor+dt at kernel.org;
> hjc at rock-chips.com; heiko at sntech.de; andy.yan at rock-chips.com; Xingyu Wu
> <xingyu.wu at starfivetech.com>; p.zabel at pengutronix.de; Jack Zhu
> <jack.zhu at starfivetech.com>; Shengyang Chen
> <shengyang.chen at starfivetech.com>; dri-devel at lists.freedesktop.org;
> devicetree at vger.kernel.org; linux-kernel at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH v4 10/10] drm/vs: add simple dsi encoder
> 
> On Sun, Jun 23, 2024 at 07:17:09AM GMT, Keith Zhao wrote:
> > Hi Dmitry:
> >
> > > On Tue, May 21, 2024 at 06:58:17PM +0800, keith wrote:
> 
> > > > +								  "starfive,syscon",
> > > > +								  2, args);
> > > > +
> > > > +	if (IS_ERR(simple->dss_regmap)) {
> > > > +		return dev_err_probe(dev, PTR_ERR(simple->dss_regmap),
> > > > +				     "getting the regmap failed\n");
> > > > +	}
> > > > +
> > > > +	simple->offset = args[0];
> > > > +	simple->mask = args[1];
> > >
> > > Is the value that you've read platform dependent or use case dependent?
> > > What is the actual value being written? Why are you using syscon for it?
> >
> > The syscon is used to select crtcs binded with encoder, If this
> > encoder binds to crtc0 , set the syscon reg bit0 = 1 If this encoder
> > binds to crtc1 , set the syscon reg bit1 = 1 (0x2) Maybe I can do this
> > by the possible_crtc instead of using args from dts
> 
> If this is a constant between your platforms, it should not be a part of DT.
> 
> >
> >
> > >
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static void vs_encoder_atomic_enable(struct drm_encoder *encoder,
> > > > +struct drm_atomic_state *state) {
> > > > +	struct vs_simple_encoder *simple = to_simple_encoder(encoder);
> > > > +
> > > > +	regmap_update_bits(simple->dss_regmap, simple->offset,
> > > > +simple->mask,
> > > > +simple->mask);
> > >
> > >
> > > A purist in me would ask to have separate mask and value to write.
> > Understand , will avoid this action
> > >
> > > > +}
> > >
> > > Is it necessary to clear those bits when stopping the stream?
> > No need to do this , if clear those bits , the encoder will point to a
> > unknown crtc
> 
> what are the consequences? Is it desirable or not?
There are two crtcs.
Each display terminal encoder can combine any crtc, depending on the value of possible crtc.
When the bit is 0, it means that the encoder matches crtc0.
When the bit is 1, it means that the encoder matches crtc1.
The possible crtc of this encoder is 2 , the reg bit is 1.    
When the video stream is stopped, if the bit is cleared, the result is that the encoder hardware points to crtc0, 
and the encoder points to crtc1 based on the drm framework(because the possible crtc no change).

> 
> --
> With best wishes
> Dmitry


More information about the dri-devel mailing list