From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751597AbbCPFDt (ORCPT ); Mon, 16 Mar 2015 01:03:49 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:52439 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750867AbbCPFDq (ORCPT ); Mon, 16 Mar 2015 01:03:46 -0400 Message-ID: <5506642C.2010103@codeaurora.org> Date: Mon, 16 Mar 2015 10:33:40 +0530 From: Archit Taneja User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: =?windows-1252?Q?St=E9phane_Viau?= 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 References: <1425407775-7704-1-git-send-email-sviau@codeaurora.org> <1425906667-3363-1-git-send-email-sviau@codeaurora.org> <1425906667-3363-6-git-send-email-sviau@codeaurora.org> <550156AA.6050005@codeaurora.org> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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 >>> --- >>> 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