linux-kernel - Re: [PATCH 5/5] drm/msm/mdp5: Add hardware configuration for msm8x16

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5506642C.2010103@codeaurora.org>
Date:	Mon, 16 Mar 2015 10:33:40 +0530
From:	Archit Taneja <architt@...eaurora.org>
To:	Stéphane Viau <sviau@...eaurora.org>
CC:	dri-devel@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org, robdclark@...il.com
Subject: Re: [PATCH 5/5] drm/msm/mdp5: Add hardware configuration for msm8x16



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@...eaurora.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/