All of lore.kernel.org
 help / color / mirror / Atom feed
From: Archit Taneja <architt@codeaurora.org>
To: "Stéphane Viau" <sviau@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 5/5] drm/msm/mdp5: Add hardware configuration for msm8x16
Date: Mon, 16 Mar 2015 10:33:40 +0530	[thread overview]
Message-ID: <5506642C.2010103@codeaurora.org> (raw)
In-Reply-To: <e520081047a461f22305cc793620c365.squirrel@www.codeaurora.org>



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

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Archit Taneja <architt@codeaurora.org>
To: "Stéphane Viau" <sviau@codeaurora.org>
Cc: dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org, robdclark@gmail.com
Subject: Re: [PATCH 5/5] drm/msm/mdp5: Add hardware configuration for msm8x16
Date: Mon, 16 Mar 2015 10:33:40 +0530	[thread overview]
Message-ID: <5506642C.2010103@codeaurora.org> (raw)
In-Reply-To: <e520081047a461f22305cc793620c365.squirrel@www.codeaurora.org>



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

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

  parent reply	other threads:[~2015-03-16  5:03 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-03 18:36 [PATCH 0/4] drm/msm: preparation for WB/DSI connectors Stephane Viau
2015-03-03 18:36 ` [PATCH 1/4] drm/msm/mdp5: Update generated header files Stephane Viau
2015-03-03 18:36   ` Stephane Viau
2015-03-03 18:37   ` [PATCH] rnndb: Prepare for more interfaces support (WB, DSI) Stephane Viau
2015-03-03 18:37     ` Stephane Viau
2015-03-03 18:36 ` [PATCH 2/4] drm/msm/mdp5: Enhance operation mode for pipeline configuration Stephane Viau
2015-03-03 18:36   ` Stephane Viau
2015-03-04  6:22   ` Archit Taneja
2015-03-04  6:22     ` Archit Taneja
2015-03-03 18:36 ` [PATCH 3/4] drm/msm/mdp5: Add START signal to kick off certain pipelines Stephane Viau
2015-03-03 18:36   ` Stephane Viau
2015-03-03 18:36 ` [PATCH 4/4] drm/msm/mdp5: Make the intf connection in config module Stephane Viau
2015-03-03 18:36   ` Stephane Viau
2015-03-04  6:13   ` Archit Taneja
2015-03-04  6:13     ` Archit Taneja
2015-03-04 15:44     ` "Stéphane Viau"
2015-03-04 15:44       ` "Stéphane Viau"
2015-03-05  4:03       ` Archit Taneja
2015-03-05 14:59 ` [PATCH v2 0/4] drm/msm: preparation for WB/DSI connectors Stephane Viau
2015-03-05 14:59   ` [PATCH v2 1/4] drm/msm/mdp5: Update generated header files Stephane Viau
2015-03-05 14:59     ` Stephane Viau
2015-03-05 14:59   ` [PATCH v2 2/4] drm/msm/mdp5: Enhance operation mode for pipeline configuration Stephane Viau
2015-03-05 14:59     ` Stephane Viau
2015-03-05 14:59   ` [PATCH v2 3/4] drm/msm/mdp5: Add START signal to kick off certain pipelines Stephane Viau
2015-03-05 14:59   ` [PATCH v2 4/4] drm/msm/mdp5: Make the intf connection in config module Stephane Viau
2015-03-05 14:59     ` Stephane Viau
2015-03-09 13:11 ` [PATCH 0/5] drm/msm: Add display configuration for msm8x16 Stephane Viau
2015-03-09 13:11   ` [PATCH 1/5] drm/msm/mdp5: Update headers (introduce MDP5 domain) Stephane Viau
2015-03-09 13:11   ` [PATCH 2/5] drm/msm/mdp5: Separate MDP5 domain from MDSS domain Stephane Viau
2015-03-09 13:11   ` [PATCH 3/5] drm/msm/mdp5: Update headers (remove enum mdp5_client_id) Stephane Viau
2015-03-09 13:11   ` [PATCH 4/5] drm/msm/mdp5: Get SMP client list from mdp5_cfg Stephane Viau
2015-03-09 13:11   ` [PATCH 5/5] drm/msm/mdp5: Add hardware configuration for msm8x16 Stephane Viau
2015-03-12  9:04     ` Archit Taneja
2015-03-12  9:04       ` Archit Taneja
2015-03-13 19:45       ` "Stéphane Viau"
2015-03-13 19:49         ` [PATCH v3 0/4] drm/msm: preparation for WB/DSI connectors Stephane Viau
2015-03-13 19:49           ` [PATCH v3 1/4] drm/msm/mdp5: Update generated header files Stephane Viau
2015-03-13 19:49             ` Stephane Viau
2015-03-13 19:49           ` [PATCH v3 2/4] drm/msm/mdp5: Enhance operation mode for pipeline configuration Stephane Viau
2015-03-13 19:49           ` [PATCH v3 3/4] drm/msm/mdp5: Add START signal to kick off certain pipelines Stephane Viau
2015-03-13 19:49             ` Stephane Viau
2015-03-23 10:50             ` Archit Taneja
2015-03-23 10:50               ` Archit Taneja
2015-03-23 22:10               ` "Stéphane Viau"
2015-03-23 22:10                 ` "Stéphane Viau"
2015-03-24  4:47                 ` Archit Taneja
2015-03-13 19:49           ` [PATCH v3 4/4] drm/msm/mdp5: Make the intf connection in config module Stephane Viau
2015-03-13 19:49             ` Stephane Viau
2015-03-16  5:03         ` Archit Taneja [this message]
2015-03-16  5:03           ` [PATCH 5/5] drm/msm/mdp5: Add hardware configuration for msm8x16 Archit Taneja
2015-03-09 13:12   ` [PATCH 1/2] rnndb: Separate MDP5 domain from MDSS domain Stephane Viau
2015-03-09 13:12     ` [PATCH 2/2] rnndb: Do not use enum mdp5_client_id to configure SMP Stephane Viau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5506642C.2010103@codeaurora.org \
    --to=architt@codeaurora.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sviau@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.