Path: csiph.com!news.mixmin.net!news.unit0.net!news.panservice.it!bofh.it!news.nic.it!robomod From: Steve Longerbeam Newsgroups: linux.kernel Subject: Re: [PATCH 06/16] gpu: ipu-v3: Add ipu_set_vdi_src_mux() Date: Fri, 15 Jul 2016 23:40:01 +0200 Message-ID: References: X-Original-To: Philipp Zabel , Steve Longerbeam User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Sender: robomod@news.nic.it List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Approved: robomod@news.nic.it Lines: 62 Organization: linux.* mail to news gateway X-Original-Cc: , X-Original-Date: Fri, 15 Jul 2016 14:33:03 -0700 X-Original-Message-ID: <5789568F.1030407@mentor.com> X-Original-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> X-Original-Sender: linux-kernel-owner@vger.kernel.org Xref: csiph.com linux.kernel:1444610 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