Re: [PATCH] drm/omap: add OMAP5 DSIPHY lane-enable support

Tomi Valkeinen
2017-08-10T08:28:44Z

[-- Attachment #1: Type: text/plain, Size: 228 bytes --]

﻿This message contains a digitally signed email which can be read by opening the attachment.
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[-- Attachment #2: Type: message/rfc822, Size: 6200 bytes --]

[-- Attachment #2.1.1.1: Type: text/plain, Size: 2891 bytes --]

Hi,

On 10/08/17 10:22, Laurent Pinchart wrote:
> Hi Tomi,
>
> Thank you for the patch.
>
> On Thursday 10 Aug 2017 09:45:20 Tomi Valkeinen wrote:
>> We are missing OMAP5 DSIPHY lane-enable support, which has prevented
>> OMAP5 DSI working in mainline. This patch adds the lane-enable similarly
>> to the recently added OMAP4 version.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>
>> Based on Laurent's recent omapdrm series.
>>
>> drivers/gpu/drm/omapdrm/dss/dsi.c | 50 +++++++++++++++++++++++++++++++----
>> 1 file changed, 45 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c
>> b/drivers/gpu/drm/omapdrm/dss/dsi.c index a66d2b1a6c74..1f0b29af9b86 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> @@ -2139,14 +2139,51 @@ static int dsi_omap4_mux_pads(struct dsi_data *dsi,
>> unsigned int lanes) return 0;
>> }
>>
>> +#define OMAP5_DSIPHY_SYSCON_OFFSET 0x74
>> +
>> +#define OMAP5_DSI1_LANEENABLE_SHIFT 24
>> +#define OMAP5_DSI2_LANEENABLE_SHIFT 19
>> +#define OMAP5_DSI_LANEENABLE_MASK 0x1f
>> +
>> +static int dsi_omap5_mux_pads(struct dsi_data *dsi, unsigned int lanes)
>> +{
>> + u32 enable_mask, enable_shift, reg;
>> +
>> + if (!dsi->syscon)
>> + return 0;
>
> The purpose of this check in dsi_omap4_mux_pads() was to handle the SoCs that
> don't require any driver action to mux the DSI pins. Now that
> dsi_enable_pads() and dsi_disable_pads() have new SoC checks, this isn't
> needed anymore, dsi->syscon can't be NULL on OMAP4 or OMAP5.

True, I'll drop the check.

>> + if (dsi->module_id == 0)
>> + enable_shift = OMAP5_DSI1_LANEENABLE_SHIFT;
>> + else if (dsi->module_id == 1)
>> + enable_shift = OMAP5_DSI2_LANEENABLE_SHIFT;
>> + else
>> + return -ENODEV;
>> +
>> + enable_mask = OMAP5_DSI_LANEENABLE_MASK << enable_shift;
>> +
>> + regmap_read(dsi->syscon, OMAP5_DSIPHY_SYSCON_OFFSET, &reg);
>> + reg &= ~enable_mask;
>> + reg |= (lanes << enable_shift) & enable_mask;
>> + regmap_write(dsi->syscon, OMAP5_DSIPHY_SYSCON_OFFSET, reg);
>> +
>> + return 0;
>> +}
>> +
>> static int dsi_enable_pads(struct dsi_data *dsi, unsigned int lane_mask)
>> {
>> - return dsi_omap4_mux_pads(dsi, lane_mask);
>> + if (dsi->data->model == DSI_MODEL_OMAP4)
>> + return dsi_omap4_mux_pads(dsi, lane_mask);
>> + if (dsi->data->model == DSI_MODEL_OMAP5)
>> + return dsi_omap5_mux_pads(dsi, lane_mask);
>> + return 0;
>> }
>>
>> static void dsi_disable_pads(struct dsi_data *dsi)
>> {
>> - dsi_omap4_mux_pads(dsi, 0);
>> + if (dsi->data->model == DSI_MODEL_OMAP4)
>> + dsi_omap4_mux_pads(dsi, 0);
>> + if (dsi->data->model == DSI_MODEL_OMAP4)
>
> Did you mean DSI_MODEL_OMAP5 ? As the two are mutually exclusive, I would use
> an "else if".

Indeed, it should be OMAP5.

Tomi

[-- Attachment #2.1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]