From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751446AbcGOVdI (ORCPT ); Fri, 15 Jul 2016 17:33:08 -0400 Received: from relay1.mentorg.com ([192.94.38.131]:39711 "EHLO relay1.mentorg.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750869AbcGOVdH (ORCPT ); Fri, 15 Jul 2016 17:33:07 -0400 Subject: Re: [PATCH 06/16] gpu: ipu-v3: Add ipu_set_vdi_src_mux() To: Philipp Zabel , Steve Longerbeam References: <1467932621-358-1-git-send-email-steve_longerbeam@mentor.com> <1467932621-358-7-git-send-email-steve_longerbeam@mentor.com> <1468586926.7213.36.camel@pengutronix.de> CC: , From: Steve Longerbeam Message-ID: <5789568F.1030407@mentor.com> Date: Fri, 15 Jul 2016 14:33:03 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <1468586926.7213.36.camel@pengutronix.de> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/15/2016 05:48 AM, Philipp Zabel wrote: > Am Donnerstag, den 07.07.2016, 16:03 -0700 schrieb Steve Longerbeam: >> Adds ipu_set_vdi_src_mux() that selects the VDIC input >> (from CSI or memory). >> >> Signed-off-by: Steve Longerbeam >> --- >> drivers/gpu/ipu-v3/ipu-common.c | 20 ++++++++++++++++++++ >> include/video/imx-ipu-v3.h | 1 + >> 2 files changed, 21 insertions(+) >> >> diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c >> index 6d1676e..374100e 100644 >> --- a/drivers/gpu/ipu-v3/ipu-common.c >> +++ b/drivers/gpu/ipu-v3/ipu-common.c >> @@ -730,6 +730,26 @@ void ipu_set_ic_src_mux(struct ipu_soc *ipu, int csi_id, bool vdi) >> } >> EXPORT_SYMBOL_GPL(ipu_set_ic_src_mux); >> >> +/* >> + * Set the source for the VDIC. Selects either from CSI[01] or memory. >> + */ >> +void ipu_set_vdi_src_mux(struct ipu_soc *ipu, bool csi) >> +{ >> + unsigned long flags; >> + u32 val; >> + >> + spin_lock_irqsave(&ipu->lock, flags); >> + >> + val = ipu_cm_read(ipu, IPU_FS_PROC_FLOW1); >> + val &= ~(0x3 << 28); >> + if (csi) >> + val |= (0x01 << 28); > This doesn't set a bus mux, or does it? As I understand it, this > configures the FSU to trigger the VDIC conversion from the CSI direct > channel sync signals (IC channel CB7). As such I believe this belongs > together with the other channel linking code, maybe even in ipu-fsu.c. It can't be folded into ipu_idmac_link(), since that links idmac channels. I don't know if this should be characterized as a mux either. But it does seem to have a similar function to setting the IC source (ipu_set_ic_src_mux()), which configures the FSU to trigger the IC conversion to start from CSI or VDIC sync signals. But it is a bit of a mystery why the VDIC source bits were placed in IPU_FS_PROC_FLOW1 instead of IPU_CONF. > > Also, some #defines would be nice, for example: > > #define FS_VDI_SRC_SEL_MASK (0x3 << 28) > #define FS_VDI_SRC_SEL_OFFSET 28 > #define FS_VDI_SRC_SEL_CSI_DIRECT 0x1 > #define FS_VDI_SRC_SEL_VDOA 0x2 Ok I will do that in next version. Steve From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Longerbeam Subject: Re: [PATCH 06/16] gpu: ipu-v3: Add ipu_set_vdi_src_mux() Date: Fri, 15 Jul 2016 14:33:03 -0700 Message-ID: <5789568F.1030407@mentor.com> References: <1467932621-358-1-git-send-email-steve_longerbeam@mentor.com> <1467932621-358-7-git-send-email-steve_longerbeam@mentor.com> <1468586926.7213.36.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1468586926.7213.36.camel@pengutronix.de> Sender: linux-kernel-owner@vger.kernel.org To: Philipp Zabel , Steve Longerbeam Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org On 07/15/2016 05:48 AM, Philipp Zabel wrote: > Am Donnerstag, den 07.07.2016, 16:03 -0700 schrieb Steve Longerbeam: >> Adds ipu_set_vdi_src_mux() that selects the VDIC input >> (from CSI or memory). >> >> Signed-off-by: Steve Longerbeam >> --- >> drivers/gpu/ipu-v3/ipu-common.c | 20 ++++++++++++++++++++ >> include/video/imx-ipu-v3.h | 1 + >> 2 files changed, 21 insertions(+) >> >> diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c >> index 6d1676e..374100e 100644 >> --- a/drivers/gpu/ipu-v3/ipu-common.c >> +++ b/drivers/gpu/ipu-v3/ipu-common.c >> @@ -730,6 +730,26 @@ void ipu_set_ic_src_mux(struct ipu_soc *ipu, int csi_id, bool vdi) >> } >> EXPORT_SYMBOL_GPL(ipu_set_ic_src_mux); >> >> +/* >> + * Set the source for the VDIC. Selects either from CSI[01] or memory. >> + */ >> +void ipu_set_vdi_src_mux(struct ipu_soc *ipu, bool csi) >> +{ >> + unsigned long flags; >> + u32 val; >> + >> + spin_lock_irqsave(&ipu->lock, flags); >> + >> + val = ipu_cm_read(ipu, IPU_FS_PROC_FLOW1); >> + val &= ~(0x3 << 28); >> + if (csi) >> + val |= (0x01 << 28); > This doesn't set a bus mux, or does it? As I understand it, this > configures the FSU to trigger the VDIC conversion from the CSI direct > channel sync signals (IC channel CB7). As such I believe this belongs > together with the other channel linking code, maybe even in ipu-fsu.c. It can't be folded into ipu_idmac_link(), since that links idmac channels. I don't know if this should be characterized as a mux either. But it does seem to have a similar function to setting the IC source (ipu_set_ic_src_mux()), which configures the FSU to trigger the IC conversion to start from CSI or VDIC sync signals. But it is a bit of a mystery why the VDIC source bits were placed in IPU_FS_PROC_FLOW1 instead of IPU_CONF. > > Also, some #defines would be nice, for example: > > #define FS_VDI_SRC_SEL_MASK (0x3 << 28) > #define FS_VDI_SRC_SEL_OFFSET 28 > #define FS_VDI_SRC_SEL_CSI_DIRECT 0x1 > #define FS_VDI_SRC_SEL_VDOA 0x2 Ok I will do that in next version. Steve