Re: [PATCH v3 1/2] drm/bridge: Add Cadence DSI driver

From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, Cyprian Wronka <cwronka@cadence.com>,
	Pawel Moll <pawel.moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Simon Hatliff <hatliff@cadence.com>,
	dri-devel@lists.freedesktop.org,
	Richard Sproul <sproul@cadence.com>,
	Alan Douglas <adouglas@cadence.com>,
	Rob Herring <robh+dt@kernel.org>, Jyri Sarha <jsarha@ti.com>,
	Kumar Gala <galak@codeaurora.org>,
	Maxime Ripard <maxime.ripard@free-electrons.com>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Neil Webb <neilw@cadence.com>
Subject: Re: [PATCH v3 1/2] drm/bridge: Add Cadence DSI driver
Date: Tue, 19 Sep 2017 15:48:14 +0200	[thread overview]
Message-ID: <20170919154814.02485818@bbrezillon> (raw)
In-Reply-To: <2ccf3816-8a98-5626-7e4e-1dd13f167811@ti.com>

On Tue, 19 Sep 2017 16:38:31 +0300
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 19/09/17 16:25, Boris Brezillon wrote:
> > On Tue, 19 Sep 2017 15:59:20 +0300
> > Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >   
> >> Hi Boris,
> >>
> >>
> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> >>
> >> On 31/08/17 18:55, Boris Brezillon wrote:  
> >>> Add a driver for Cadence DPI -> DSI bridge.
> >>>
> >>> This driver only support a subset of Cadence DSI bridge capabilities.
> >>>
> >>> Here is a non-exhaustive list of missing features:
> >>>  * burst mode
> >>>  * dynamic configuration of the DPHY based on the
> >>>  * support for additional input interfaces (SDI input)
> >>>
> >>> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>    
> >>
> >> <snip>
> >>  
> >>> +	dsi->pclk = devm_clk_get(&pdev->dev, "pclk");
> >>> +	if (IS_ERR(dsi->pclk))
> >>> +		return PTR_ERR(dsi->pclk);    
> >>
> >> What's the purpose of pclk? Isn't that normally dealt with the normal
> >> modesetting, enabled with the video stream? How could it even be enabled
> >> here, without anyone setting the rate?  
> > 
> > It's the peripheral clock, not the pixel clock, and AFAIU it has to be
> > enabled before accessing DSI registers.  
> 
> Is that the dsi_p_clk? I can't find "peripheral clock" in the specs.

Yep, it is dsi_p_clk (the APB clock).

> 
> I think calling it "pclk" in a display driver is very confusing, as
> pclk, at least for me, always means pixel clock =).

I can rename it if you prefer. What name would you like to see?
abp_clk? periph_clk? Something else?

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel