diff mbox

[5/5] drm/msm/mdp5: Add hardware configuration for msm8x16

Message ID 1425906667-3363-6-git-send-email-sviau@codeaurora.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Stephane Viau March 9, 2015, 1:11 p.m. UTC
This change adds the hw configuration for msm8x16 chipsets in
mdp5_cfg module.

Note that only one external display interface is present in this
configuration (DSI) but has not been enabled yet. It will be enabled
once drm/msm driver supports DSI connectors.

Signed-off-by: Stephane Viau <sviau@codeaurora.org>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 51 ++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

Comments

Archit Taneja March 12, 2015, 9:04 a.m. UTC | #1
Hi,

On 03/09/2015 06:41 PM, Stephane Viau wrote:
> This change adds the hw configuration for msm8x16 chipsets in
> mdp5_cfg module.
>
> Note that only one external display interface is present in this
> configuration (DSI) but has not been enabled yet. It will be enabled
> once drm/msm driver supports DSI connectors.
>
> Signed-off-by: Stephane Viau <sviau@codeaurora.org>
> ---
>   drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 51 ++++++++++++++++++++++++++++++++-
>   1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> index 96ea6dd..9ff7ac1 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
> @@ -1,5 +1,5 @@
>   /*
> - * Copyright (c) 2014 The Linux Foundation. All rights reserved.
> + * Copyright (c) 2014-2015 The Linux Foundation. All rights reserved.
>    *
>    * This program is free software; you can redistribute it and/or modify
>    * it under the terms of the GNU General Public License version 2 and
> @@ -150,10 +150,59 @@ const struct mdp5_cfg_hw apq8084_config = {
>   	.max_clk = 320000000,
>   };
>
> +const struct mdp5_cfg_hw msm8x16_config = {
> +	.name = "msm8x16",
> +	.mdp = {
> +		.count = 1,
> +		.base = { 0x01000 },
> +	},
> +	.smp = {
> +		.mmb_count = 8,
> +		.mmb_size = 8192,
> +		.clients = {
> +			[SSPP_VIG0] = 1, [SSPP_DMA0] = 4,
> +			[SSPP_RGB0] = 7, [SSPP_RGB1] = 8,
> +		},
> +	},
> +	.ctl = {
> +		.count = 5,
> +		.base = { 0x02000, 0x02200, 0x02400, 0x02600, 0x02800 },
> +	},
> +	.pipe_vig = {
> +		.count = 1,
> +		.base = { 0x05000 },
> +	},
> +	.pipe_rgb = {
> +		.count = 2,
> +		.base = { 0x15000, 0x17000 },
> +	},
> +	.pipe_dma = {
> +		.count = 1,
> +		.base = { 0x25000 },
> +	},
> +	.lm = {
> +		.count = 2, /* LM0 and LM3 */
> +		.base = { 0x45000, 0x48000 },
> +		.nb_stages = 5,
> +	},
> +	.dspp = {
> +		.count = 1,
> +		.base = { 0x55000 },
> +
> +	},
> +	.intf = {
> +		.count = 1, /* INTF_1 */
> +		.base = { 0x6B800 },

We would need to put the other non-existent INTF_0, INTF_2 and INTF_3 
base addresses here too, so that the writes to 
REG_MDP5_INTF_TIMING_ENGINE_EN in the patch "Make the intf connection in 
config module" access the correct registers.

Archit

> +	},
> +	/* TODO enable .intfs[] with [1] = INTF_DSI, once DSI is implemented */
> +	.max_clk = 320000000,
> +};
> +
>   static const struct mdp5_cfg_handler cfg_handlers[] = {
>   	{ .revision = 0, .config = { .hw = &msm8x74_config } },
>   	{ .revision = 2, .config = { .hw = &msm8x74_config } },
>   	{ .revision = 3, .config = { .hw = &apq8084_config } },
> +	{ .revision = 6, .config = { .hw = &msm8x16_config } },
>   };
>
>
>
Stephane Viau March 13, 2015, 7:45 p.m. UTC | #2
Hi,

> Hi,
>
> On 03/09/2015 06:41 PM, Stephane Viau wrote:
>> This change adds the hw configuration for msm8x16 chipsets in
>> mdp5_cfg module.
>>
>> Note that only one external display interface is present in this
>> configuration (DSI) but has not been enabled yet. It will be enabled
>> once drm/msm driver supports DSI connectors.
>>
>> Signed-off-by: Stephane Viau <sviau@codeaurora.org>
>> ---
>>   drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 51
>> ++++++++++++++++++++++++++++++++-
>>   1 file changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
>> index 96ea6dd..9ff7ac1 100644
>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
>> @@ -1,5 +1,5 @@
>>   /*
>> - * Copyright (c) 2014 The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2014-2015 The Linux Foundation. All rights reserved.
>>    *
>>    * This program is free software; you can redistribute it and/or
>> modify
>>    * it under the terms of the GNU General Public License version 2 and
>> @@ -150,10 +150,59 @@ const struct mdp5_cfg_hw apq8084_config = {
>>   	.max_clk = 320000000,
>>   };
>>
>> +const struct mdp5_cfg_hw msm8x16_config = {
>> +	.name = "msm8x16",
>> +	.mdp = {
>> +		.count = 1,
>> +		.base = { 0x01000 },
>> +	},
>> +	.smp = {
>> +		.mmb_count = 8,
>> +		.mmb_size = 8192,
>> +		.clients = {
>> +			[SSPP_VIG0] = 1, [SSPP_DMA0] = 4,
>> +			[SSPP_RGB0] = 7, [SSPP_RGB1] = 8,
>> +		},
>> +	},
>> +	.ctl = {
>> +		.count = 5,
>> +		.base = { 0x02000, 0x02200, 0x02400, 0x02600, 0x02800 },
>> +	},
>> +	.pipe_vig = {
>> +		.count = 1,
>> +		.base = { 0x05000 },
>> +	},
>> +	.pipe_rgb = {
>> +		.count = 2,
>> +		.base = { 0x15000, 0x17000 },
>> +	},
>> +	.pipe_dma = {
>> +		.count = 1,
>> +		.base = { 0x25000 },
>> +	},
>> +	.lm = {
>> +		.count = 2, /* LM0 and LM3 */
>> +		.base = { 0x45000, 0x48000 },
>> +		.nb_stages = 5,
>> +	},
>> +	.dspp = {
>> +		.count = 1,
>> +		.base = { 0x55000 },
>> +
>> +	},
>> +	.intf = {
>> +		.count = 1, /* INTF_1 */
>> +		.base = { 0x6B800 },
>
> We would need to put the other non-existent INTF_0, INTF_2 and INTF_3
> base addresses here too, so that the writes to
> REG_MDP5_INTF_TIMING_ENGINE_EN in the patch "Make the intf connection in
> config module" access the correct registers.

You are referring here to the discussion we had in "drm/msm/mdp5: Make the
intf connection in config module"[1]...

Let me clarify:
We see these faults when interfaces are *present* but not *supported* by
the driver, where:
 - *present* means that the interfaces are physically implemented in HW
and therefore need to be reflected by "intf.base" addresses
 - *supported* means that the msm KMS driver is actually able to drive an
interface
If you look at mdp5_cfg.c in "drm/msm/mdp5: Make the intf connection in
config module"[1], 4 interfaces (intf.base) are present (for apq8084), but
only 2 are supported (intfs[])...
What I meant is that even though our driver cannot support all interfaces
present in a chip, we still need to disable them in mdp5_kms_init().
(BTW, this is probably due to my bootloader enabling the DSI whereas the
msm KMS driver does not support it as of yet - and thus leads to
artifacts/underflow on my eDP panel.)

So to answer your comment, it doesn't make too much sense to define base
addresses for interfaces that are not present in a chip. Instead I'd
prefer to check if intf.base is not NULL before writing in
REG_MDP5_INTF_TIMING_ENGINE_EN registers.. and avoid some INVALID_INDEX
BUGs ;-).

I'm sending out a v3 for "drm/msm/mdp5: Make the intf connection in config
module" in a minute...

[1] http://lists.freedesktop.org/archives/dri-devel/2015-March/078651.html

>
> Archit
>
>> +	},
>> +	/* TODO enable .intfs[] with [1] = INTF_DSI, once DSI is implemented
>> */
>> +	.max_clk = 320000000,
>> +};
>> +
>>   static const struct mdp5_cfg_handler cfg_handlers[] = {
>>   	{ .revision = 0, .config = { .hw = &msm8x74_config } },
>>   	{ .revision = 2, .config = { .hw = &msm8x74_config } },
>>   	{ .revision = 3, .config = { .hw = &apq8084_config } },
>> +	{ .revision = 6, .config = { .hw = &msm8x16_config } },
>>   };
>>
>>
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Stephane Viau March 13, 2015, 7:49 p.m. UTC | #3
WB and DSI support are in the pipe and will come out soon. Before that,
we need to prepare the MDP5 driver so we can support these connectors.

v2: rename macro to mdp5_cfg_intf_is_virtual() [pointed by Archit]
v3: add sanity check before writing in INTF_TIMING_ENGINE_EN registers

Note:
"drm/msm: Add display configuration for msm8x16" patch set depends on
"drm/msm: preparation for WB/DSI connectors" patch set.

Stephane Viau (4):
  drm/msm/mdp5: Update generated header files
  drm/msm/mdp5: Enhance operation mode for pipeline configuration
  drm/msm/mdp5: Add START signal to kick off certain pipelines
  drm/msm/mdp5: Make the intf connection in config module

 drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h     |  68 +++---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c     |  10 +
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h     |  15 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c    |  70 ++----
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c     | 326 +++++++++++++++++++++++++---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h     |  75 +++----
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c |  40 ++--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c     | 100 +++++----
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h     |  49 ++++-
 9 files changed, 522 insertions(+), 231 deletions(-)
Archit Taneja March 16, 2015, 5:03 a.m. UTC | #4
On 03/14/2015 01:15 AM, "Stéphane Viau" wrote:
> Hi,
>
>> Hi,
>>
>> On 03/09/2015 06:41 PM, Stephane Viau wrote:
>>> This change adds the hw configuration for msm8x16 chipsets in
>>> mdp5_cfg module.
>>>
>>> Note that only one external display interface is present in this
>>> configuration (DSI) but has not been enabled yet. It will be enabled
>>> once drm/msm driver supports DSI connectors.
>>>
>>> Signed-off-by: Stephane Viau <sviau@codeaurora.org>
>>> ---
>>>    drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 51
>>> ++++++++++++++++++++++++++++++++-
>>>    1 file changed, 50 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
>>> b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
>>> index 96ea6dd..9ff7ac1 100644
>>> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
>>> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
>>> @@ -1,5 +1,5 @@
>>>    /*
>>> - * Copyright (c) 2014 The Linux Foundation. All rights reserved.
>>> + * Copyright (c) 2014-2015 The Linux Foundation. All rights reserved.
>>>     *
>>>     * This program is free software; you can redistribute it and/or
>>> modify
>>>     * it under the terms of the GNU General Public License version 2 and
>>> @@ -150,10 +150,59 @@ const struct mdp5_cfg_hw apq8084_config = {
>>>    	.max_clk = 320000000,
>>>    };
>>>
>>> +const struct mdp5_cfg_hw msm8x16_config = {
>>> +	.name = "msm8x16",
>>> +	.mdp = {
>>> +		.count = 1,
>>> +		.base = { 0x01000 },
>>> +	},
>>> +	.smp = {
>>> +		.mmb_count = 8,
>>> +		.mmb_size = 8192,
>>> +		.clients = {
>>> +			[SSPP_VIG0] = 1, [SSPP_DMA0] = 4,
>>> +			[SSPP_RGB0] = 7, [SSPP_RGB1] = 8,
>>> +		},
>>> +	},
>>> +	.ctl = {
>>> +		.count = 5,
>>> +		.base = { 0x02000, 0x02200, 0x02400, 0x02600, 0x02800 },
>>> +	},
>>> +	.pipe_vig = {
>>> +		.count = 1,
>>> +		.base = { 0x05000 },
>>> +	},
>>> +	.pipe_rgb = {
>>> +		.count = 2,
>>> +		.base = { 0x15000, 0x17000 },
>>> +	},
>>> +	.pipe_dma = {
>>> +		.count = 1,
>>> +		.base = { 0x25000 },
>>> +	},
>>> +	.lm = {
>>> +		.count = 2, /* LM0 and LM3 */
>>> +		.base = { 0x45000, 0x48000 },
>>> +		.nb_stages = 5,
>>> +	},
>>> +	.dspp = {
>>> +		.count = 1,
>>> +		.base = { 0x55000 },
>>> +
>>> +	},
>>> +	.intf = {
>>> +		.count = 1, /* INTF_1 */
>>> +		.base = { 0x6B800 },
>>
>> We would need to put the other non-existent INTF_0, INTF_2 and INTF_3
>> base addresses here too, so that the writes to
>> REG_MDP5_INTF_TIMING_ENGINE_EN in the patch "Make the intf connection in
>> config module" access the correct registers.
>
> You are referring here to the discussion we had in "drm/msm/mdp5: Make the
> intf connection in config module"[1]...
>
> Let me clarify:
> We see these faults when interfaces are *present* but not *supported* by
> the driver, where:
>   - *present* means that the interfaces are physically implemented in HW
> and therefore need to be reflected by "intf.base" addresses
>   - *supported* means that the msm KMS driver is actually able to drive an
> interface
> If you look at mdp5_cfg.c in "drm/msm/mdp5: Make the intf connection in
> config module"[1], 4 interfaces (intf.base) are present (for apq8084), but
> only 2 are supported (intfs[])...
> What I meant is that even though our driver cannot support all interfaces
> present in a chip, we still need to disable them in mdp5_kms_init().
> (BTW, this is probably due to my bootloader enabling the DSI whereas the
> msm KMS driver does not support it as of yet - and thus leads to
> artifacts/underflow on my eDP panel.)

Okay, I get it now. I think I wasn't sure about whether INTF_DISABLED 
represents *present* or *supported*. Thanks for the clarification.

>
> So to answer your comment, it doesn't make too much sense to define base
> addresses for interfaces that are not present in a chip. Instead I'd
> prefer to check if intf.base is not NULL before writing in
> REG_MDP5_INTF_TIMING_ENGINE_EN registers.. and avoid some INVALID_INDEX
> BUGs ;-).

That sounds good. Thanks.

Archit
diff mbox

Patch

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
index 96ea6dd..9ff7ac1 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2014 The Linux Foundation. All rights reserved.
+ * Copyright (c) 2014-2015 The Linux Foundation. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 and
@@ -150,10 +150,59 @@  const struct mdp5_cfg_hw apq8084_config = {
 	.max_clk = 320000000,
 };
 
+const struct mdp5_cfg_hw msm8x16_config = {
+	.name = "msm8x16",
+	.mdp = {
+		.count = 1,
+		.base = { 0x01000 },
+	},
+	.smp = {
+		.mmb_count = 8,
+		.mmb_size = 8192,
+		.clients = {
+			[SSPP_VIG0] = 1, [SSPP_DMA0] = 4,
+			[SSPP_RGB0] = 7, [SSPP_RGB1] = 8,
+		},
+	},
+	.ctl = {
+		.count = 5,
+		.base = { 0x02000, 0x02200, 0x02400, 0x02600, 0x02800 },
+	},
+	.pipe_vig = {
+		.count = 1,
+		.base = { 0x05000 },
+	},
+	.pipe_rgb = {
+		.count = 2,
+		.base = { 0x15000, 0x17000 },
+	},
+	.pipe_dma = {
+		.count = 1,
+		.base = { 0x25000 },
+	},
+	.lm = {
+		.count = 2, /* LM0 and LM3 */
+		.base = { 0x45000, 0x48000 },
+		.nb_stages = 5,
+	},
+	.dspp = {
+		.count = 1,
+		.base = { 0x55000 },
+
+	},
+	.intf = {
+		.count = 1, /* INTF_1 */
+		.base = { 0x6B800 },
+	},
+	/* TODO enable .intfs[] with [1] = INTF_DSI, once DSI is implemented */
+	.max_clk = 320000000,
+};
+
 static const struct mdp5_cfg_handler cfg_handlers[] = {
 	{ .revision = 0, .config = { .hw = &msm8x74_config } },
 	{ .revision = 2, .config = { .hw = &msm8x74_config } },
 	{ .revision = 3, .config = { .hw = &apq8084_config } },
+	{ .revision = 6, .config = { .hw = &msm8x16_config } },
 };