From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759041AbcG1CIG (ORCPT ); Wed, 27 Jul 2016 22:08:06 -0400 Received: from mailgw01.mediatek.com ([210.61.82.183]:7855 "EHLO mailgw01.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1758794AbcG1CH5 (ORCPT ); Wed, 27 Jul 2016 22:07:57 -0400 Message-ID: <1469671655.11841.85.camel@mtksdaap41> Subject: Re: [PATCH v4 4/8] drm/mediatek: add support for Mediatek SoC MT2701 From: CK Hu To: YT Shen CC: Philipp Zabel , , Rob Herring , Pawel Moll , "Mark Rutland" , Ian Campbell , Kumar Gala , "Russell King" , David Airlie , "Matthias Brugger" , Mao Huang , "Bibby Hsieh" , , , , , , "Sascha Hauer" , , , Date: Thu, 28 Jul 2016 10:07:35 +0800 In-Reply-To: <1469613816.2470.50.camel@pengutronix.de> References: <1468577274-6178-1-git-send-email-yt.shen@mediatek.com> <1468577274-6178-5-git-send-email-yt.shen@mediatek.com> <1468997599.11841.49.camel@mtksdaap41> <1469529774.4678.34.camel@mtksdaap41> <1469613816.2470.50.camel@pengutronix.de> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit MIME-Version: 1.0 X-MTK: N Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, YT: On Wed, 2016-07-27 at 12:03 +0200, Philipp Zabel wrote: > Am Dienstag, den 26.07.2016, 18:42 +0800 schrieb YT Shen: > > Hi CK, > > > > On Wed, 2016-07-20 at 14:53 +0800, CK Hu wrote: > > > Hi, YT: > > > > > > Some comments inline. > > > > > > On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote: > > > > This patch add support for the Mediatek MT2701 DISP subsystem. > > > > There is only one OVL engine in MT2701. > > > > > > > > Signed-off-by: YT Shen > > > > --- > > > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++ > > > > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 ++++ > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 41 +++++++++++++++++++++++++++ > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 7 +++++ > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + > > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 31 ++++++++++++++++++++ > > > > 6 files changed, 92 insertions(+) > > > > > > > > > > [snip...] > > > > > > > > > > > static const struct of_device_id mtk_ddp_comp_dt_ids[] = { > > > > + { .compatible = "mediatek,mt2701-disp-ovl", .data = (void *)MTK_DISP_OVL }, > > > > { .compatible = "mediatek,mt8173-disp-ovl", .data = (void *)MTK_DISP_OVL }, > > > > + { .compatible = "mediatek,mt2701-disp-rdma", .data = (void *)MTK_DISP_RDMA }, > > > > { .compatible = "mediatek,mt8173-disp-rdma", .data = (void *)MTK_DISP_RDMA }, > > > > + { .compatible = "mediatek,mt2701-disp-wdma", .data = (void *)MTK_DISP_WDMA }, > > > > > > Is WDMA different from MT8173 to MT2701. If they are the same, you need > > > not to add compatible of 'mediatek,mt2701-disp-wdma' because use > > > 'mediatek,mt8173-disp-wdma' is enough. > > Yes, the hardware has differences, so we need add new compatible > > 'mt2701-disp-wdma' > > It would be nice if you could add a short paragraph to the patch > description about what the hardware differences are (probably just > changed register layout in most cases). If you don't want to add description, you may separate the modification of OVL, RDMA, WDMA, PWM to different patch. And you send patch of OVL and RDMA first because you have related code modification for these two. You temporarily treat WDMA and PWM as MT8173 and it works. And one day you need to modify code of WDMA and PWM for MT2701, then you send the patch of WDMA and PWM and you need not to add any description because we can find out the difference from source code. > > regards > Philipp > Regards, CK From mboxrd@z Thu Jan 1 00:00:00 1970 From: CK Hu Subject: Re: [PATCH v4 4/8] drm/mediatek: add support for Mediatek SoC MT2701 Date: Thu, 28 Jul 2016 10:07:35 +0800 Message-ID: <1469671655.11841.85.camel@mtksdaap41> References: <1468577274-6178-1-git-send-email-yt.shen@mediatek.com> <1468577274-6178-5-git-send-email-yt.shen@mediatek.com> <1468997599.11841.49.camel@mtksdaap41> <1469529774.4678.34.camel@mtksdaap41> <1469613816.2470.50.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1469613816.2470.50.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: YT Shen Cc: Philipp Zabel , dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , David Airlie , Matthias Brugger , Mao Huang , Bibby Hsieh , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, srv_heupstream-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org, Sascha Hauer , yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org, emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org List-Id: devicetree@vger.kernel.org Hi, YT: On Wed, 2016-07-27 at 12:03 +0200, Philipp Zabel wrote: > Am Dienstag, den 26.07.2016, 18:42 +0800 schrieb YT Shen: > > Hi CK, > > > > On Wed, 2016-07-20 at 14:53 +0800, CK Hu wrote: > > > Hi, YT: > > > > > > Some comments inline. > > > > > > On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote: > > > > This patch add support for the Mediatek MT2701 DISP subsystem. > > > > There is only one OVL engine in MT2701. > > > > > > > > Signed-off-by: YT Shen > > > > --- > > > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++ > > > > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 ++++ > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 41 +++++++++++++++++++++++++++ > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 7 +++++ > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + > > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 31 ++++++++++++++++++++ > > > > 6 files changed, 92 insertions(+) > > > > > > > > > > [snip...] > > > > > > > > > > > static const struct of_device_id mtk_ddp_comp_dt_ids[] = { > > > > + { .compatible = "mediatek,mt2701-disp-ovl", .data = (void *)MTK_DISP_OVL }, > > > > { .compatible = "mediatek,mt8173-disp-ovl", .data = (void *)MTK_DISP_OVL }, > > > > + { .compatible = "mediatek,mt2701-disp-rdma", .data = (void *)MTK_DISP_RDMA }, > > > > { .compatible = "mediatek,mt8173-disp-rdma", .data = (void *)MTK_DISP_RDMA }, > > > > + { .compatible = "mediatek,mt2701-disp-wdma", .data = (void *)MTK_DISP_WDMA }, > > > > > > Is WDMA different from MT8173 to MT2701. If they are the same, you need > > > not to add compatible of 'mediatek,mt2701-disp-wdma' because use > > > 'mediatek,mt8173-disp-wdma' is enough. > > Yes, the hardware has differences, so we need add new compatible > > 'mt2701-disp-wdma' > > It would be nice if you could add a short paragraph to the patch > description about what the hardware differences are (probably just > changed register layout in most cases). If you don't want to add description, you may separate the modification of OVL, RDMA, WDMA, PWM to different patch. And you send patch of OVL and RDMA first because you have related code modification for these two. You temporarily treat WDMA and PWM as MT8173 and it works. And one day you need to modify code of WDMA and PWM for MT2701, then you send the patch of WDMA and PWM and you need not to add any description because we can find out the difference from source code. > > regards > Philipp > Regards, CK -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: ck.hu@mediatek.com (CK Hu) Date: Thu, 28 Jul 2016 10:07:35 +0800 Subject: [PATCH v4 4/8] drm/mediatek: add support for Mediatek SoC MT2701 In-Reply-To: <1469613816.2470.50.camel@pengutronix.de> References: <1468577274-6178-1-git-send-email-yt.shen@mediatek.com> <1468577274-6178-5-git-send-email-yt.shen@mediatek.com> <1468997599.11841.49.camel@mtksdaap41> <1469529774.4678.34.camel@mtksdaap41> <1469613816.2470.50.camel@pengutronix.de> Message-ID: <1469671655.11841.85.camel@mtksdaap41> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, YT: On Wed, 2016-07-27 at 12:03 +0200, Philipp Zabel wrote: > Am Dienstag, den 26.07.2016, 18:42 +0800 schrieb YT Shen: > > Hi CK, > > > > On Wed, 2016-07-20 at 14:53 +0800, CK Hu wrote: > > > Hi, YT: > > > > > > Some comments inline. > > > > > > On Fri, 2016-07-15 at 18:07 +0800, YT Shen wrote: > > > > This patch add support for the Mediatek MT2701 DISP subsystem. > > > > There is only one OVL engine in MT2701. > > > > > > > > Signed-off-by: YT Shen > > > > --- > > > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 6 ++++ > > > > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 ++++ > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 41 +++++++++++++++++++++++++++ > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 7 +++++ > > > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + > > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 31 ++++++++++++++++++++ > > > > 6 files changed, 92 insertions(+) > > > > > > > > > > [snip...] > > > > > > > > > > > static const struct of_device_id mtk_ddp_comp_dt_ids[] = { > > > > + { .compatible = "mediatek,mt2701-disp-ovl", .data = (void *)MTK_DISP_OVL }, > > > > { .compatible = "mediatek,mt8173-disp-ovl", .data = (void *)MTK_DISP_OVL }, > > > > + { .compatible = "mediatek,mt2701-disp-rdma", .data = (void *)MTK_DISP_RDMA }, > > > > { .compatible = "mediatek,mt8173-disp-rdma", .data = (void *)MTK_DISP_RDMA }, > > > > + { .compatible = "mediatek,mt2701-disp-wdma", .data = (void *)MTK_DISP_WDMA }, > > > > > > Is WDMA different from MT8173 to MT2701. If they are the same, you need > > > not to add compatible of 'mediatek,mt2701-disp-wdma' because use > > > 'mediatek,mt8173-disp-wdma' is enough. > > Yes, the hardware has differences, so we need add new compatible > > 'mt2701-disp-wdma' > > It would be nice if you could add a short paragraph to the patch > description about what the hardware differences are (probably just > changed register layout in most cases). If you don't want to add description, you may separate the modification of OVL, RDMA, WDMA, PWM to different patch. And you send patch of OVL and RDMA first because you have related code modification for these two. You temporarily treat WDMA and PWM as MT8173 and it works. And one day you need to modify code of WDMA and PWM for MT2701, then you send the patch of WDMA and PWM and you need not to add any description because we can find out the difference from source code. > > regards > Philipp > Regards, CK