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

From: Archit Taneja <architt@codeaurora.org>
To: Boris Brezillon <boris.brezillon@bootlin.com>
Cc: "Menon, Nishanth" <nm@ti.com>,
	Mark Rutland <mark.rutland@arm.com>,
	David Airlie <airlied@linux.ie>,
	dri-devel@lists.freedesktop.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Alan Douglas <adouglas@cadence.com>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Richard Sproul <sproul@cadence.com>,
	devicetree@vger.kernel.org, Rafal Ciepiela <rafalc@cadence.com>,
	Pawel Moll <pawel.moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Simon Hatliff <hatliff@cadence.com>, Jyri Sarha <jsarha@ti.com>,
	Rob Herring <robh+dt@kernel.org>,
	Suresh Punnoose <sureshp@cadence.com>,
	Kumar Gala <galak@codeaurora.org>
Subject: Re: [PATCH v6 1/2] drm/bridge: Add Cadence DSI driver
Date: Mon, 23 Apr 2018 13:56:42 +0530	[thread overview]
Message-ID: <7d4ddac2-1df2-c4a3-6b11-8418ebbb97b7@codeaurora.org> (raw)
In-Reply-To: <20180421082017.4738b65d@bbrezillon>

On Saturday 21 April 2018 11:50 AM, Boris Brezillon wrote:
> Hi Archit,
> 
> On Sun, 15 Apr 2018 13:39:44 +0530
> Archit Taneja <architt@codeaurora.org> wrote:
> 
>>> +static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
>>> +				     struct cdns_dphy_cfg *cfg,
>>> +				     unsigned int dpi_htotal,
>>> +				     unsigned int dpi_bpp,
>>> +				     unsigned int dpi_hz,
>>> +				     unsigned int dsi_htotal,
>>> +				     unsigned int dsi_nlanes,
>>> +				     unsigned int *dsi_hfp_ext)
>>> +{
>>> +	u64 dlane_bps, dlane_bps_max, fbdiv, fbdiv_max, adj_dsi_htotal;
>>> +	unsigned long pll_ref_hz = clk_get_rate(dphy->pll_ref_clk);
>>> +
>>> +	memset(cfg, 0, sizeof(*cfg));
>>> +
>>> +	cfg->nlanes = dsi_nlanes;
>>> +
>>> +	if (pll_ref_hz < 9600000 || pll_ref_hz >= 150000000)
>>> +		return -EINVAL;
>>> +	else if (pll_ref_hz < 19200000)
>>> +		cfg->pll_ipdiv = 1;
>>> +	else if (pll_ref_hz < 38400000)
>>> +		cfg->pll_ipdiv = 2;
>>> +	else if (pll_ref_hz < 76800000)
>>> +		cfg->pll_ipdiv = 4;
>>> +	else
>>> +		cfg->pll_ipdiv = 8;
>>> +
>>> +	/*
>>> +	 * Make sure DSI htotal is aligned on a lane boundary when calculating
>>> +	 * the expected data rate. This is done by extending HFP in case of
>>> +	 * misalignment.
>>> +	 */
>>> +	adj_dsi_htotal = dsi_htotal;
>>> +	if (dsi_htotal % dsi_nlanes)
>>> +		adj_dsi_htotal += dsi_nlanes - (dsi_htotal % dsi_nlanes);
>>> +
>>> +	dlane_bps = (u64)dpi_hz * adj_dsi_htotal;
>>> +
>>> +	/* data rate in bytes/sec is not an integer, refuse the mode. */
>>> +	if (do_div(dlane_bps, dsi_nlanes * dpi_htotal))
>>> +		return -EINVAL;
>>> +
>>> +	/* data rate was in bytes/sec, convert to bits/sec. */
>>> +	dlane_bps *= 8;
>>> +
>>> +	if (dlane_bps > 2500000000UL || dlane_bps < 160000000UL)
>>> +		return -EINVAL;
>>> +	else if (dlane_bps >= 1250000000)
>>> +		cfg->pll_opdiv = 1;
>>> +	else if (dlane_bps >= 630000000)
>>> +		cfg->pll_opdiv = 2;
>>> +	else if (dlane_bps >= 320000000)
>>> +		cfg->pll_opdiv = 4;
>>> +	else if (dlane_bps >= 160000000)
>>> +		cfg->pll_opdiv = 8;
>>> +
>>> +	/*
>>> +	 * Allow a deviation of 0.2% on the per-lane data rate to try to
>>> +	 * recover a potential mismatch between DPI and PPI clks.
>>> +	 */
>>> +	dlane_bps_max = dlane_bps + (dlane_bps / 500);
>>
>> kbuild reported an error for 32 bit archs. I'm guessing it's because of
>> this divide above?
> 
> Yep, DIV_ROUND_UP_ULL() should fix the problem.
> 
>>
>>
>>> +
>>> +	fbdiv_max = DIV_ROUND_DOWN_ULL((dlane_bps + (dlane_bps / 500)) * 2 *
>>
>> You could use dlane_bps_max here instead.
> 
> Sure.
> 
>>
>> Otherwise,
>>
>> Reviewed-by: Archit Taneja <architt@codeaurora.org>
> 
> Thanks for the review.
> 
> Does that mean I'm allowed to apply the patch to drm-misc-next myself
> after fixing the div64 issue?

Sure, please go ahead.

Archit

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