From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 484E2C433DF for ; Fri, 9 Oct 2020 01:05:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BF60D22256 for ; Fri, 9 Oct 2020 01:05:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="EXXCVFqY" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728703AbgJIBFl (ORCPT ); Thu, 8 Oct 2020 21:05:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42128 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726348AbgJIBFl (ORCPT ); Thu, 8 Oct 2020 21:05:41 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D4E9BC0613D2 for ; Thu, 8 Oct 2020 18:05:40 -0700 (PDT) Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4838559E; Fri, 9 Oct 2020 03:05:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1602205539; bh=77MvvXC/iqjJHodikpSn8B9ibAjdrZV11eLQsEvucAg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=EXXCVFqYdjr11iv0uTM4QEPGPdx28D3TDjh1sSQ/RmYgiIdHUs0XULgVC9LhO/NhL tOEuUZ2XDR8K9AxADXsrCICGj77/QOMLugbW8t9brw7lk3tuwz4ZBhKJD223k+A8Q+ DQ/tH6Y31wSD0+ldmXbxqkmMybIlhE0IDYTQl4VE= Date: Fri, 9 Oct 2020 04:04:56 +0300 From: Laurent Pinchart To: Andy Shevchenko Cc: linux-media@vger.kernel.org, Yong Zhi , Sakari Ailus , Bingbu Cao , Tian Shu Qiu , Mauro Carvalho Chehab Subject: Re: [PATCH v2 03/10] media: ipu2-cio2: Replace custom definition with PAGE_SIZE Message-ID: <20201009010456.GF12857@pendragon.ideasonboard.com> References: <20200817160734.12402-1-andriy.shevchenko@linux.intel.com> <20200817160734.12402-3-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200817160734.12402-3-andriy.shevchenko@linux.intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi Andy, Thank you for the patch. On Mon, Aug 17, 2020 at 07:07:26PM +0300, Andy Shevchenko wrote: > It's quite unlikely that another page size will be supported, > but in any case there is still an inconsistency between custom > page size definition and generic macros used in the driver. > > Switch over to the generic PAGE_SIZE for sake of the consistency. Is this conceptually correct though ? Does the CIO2 have an intrinsic page size, or do pages here always refer to system memory pages ? In the latter case the change is good, otherwise a separate macro seems best. > Signed-off-by: Andy Shevchenko > --- > v2: no change > drivers/media/pci/intel/ipu3/ipu3-cio2.c | 30 ++++++++++-------------- > drivers/media/pci/intel/ipu3/ipu3-cio2.h | 1 - > 2 files changed, 13 insertions(+), 18 deletions(-) > > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > index a89cb3c7e0dc..0cb5461bfb1e 100644 > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c > @@ -96,12 +96,12 @@ static inline u32 cio2_bytesperline(const unsigned int width) > static void cio2_fbpt_exit_dummy(struct cio2_device *cio2) > { > if (cio2->dummy_lop) { > - dma_free_coherent(&cio2->pci_dev->dev, CIO2_PAGE_SIZE, > + dma_free_coherent(&cio2->pci_dev->dev, PAGE_SIZE, > cio2->dummy_lop, cio2->dummy_lop_bus_addr); > cio2->dummy_lop = NULL; > } > if (cio2->dummy_page) { > - dma_free_coherent(&cio2->pci_dev->dev, CIO2_PAGE_SIZE, > + dma_free_coherent(&cio2->pci_dev->dev, PAGE_SIZE, > cio2->dummy_page, cio2->dummy_page_bus_addr); > cio2->dummy_page = NULL; > } > @@ -111,12 +111,10 @@ static int cio2_fbpt_init_dummy(struct cio2_device *cio2) > { > unsigned int i; > > - cio2->dummy_page = dma_alloc_coherent(&cio2->pci_dev->dev, > - CIO2_PAGE_SIZE, > + cio2->dummy_page = dma_alloc_coherent(&cio2->pci_dev->dev, PAGE_SIZE, > &cio2->dummy_page_bus_addr, > GFP_KERNEL); > - cio2->dummy_lop = dma_alloc_coherent(&cio2->pci_dev->dev, > - CIO2_PAGE_SIZE, > + cio2->dummy_lop = dma_alloc_coherent(&cio2->pci_dev->dev, PAGE_SIZE, > &cio2->dummy_lop_bus_addr, > GFP_KERNEL); > if (!cio2->dummy_page || !cio2->dummy_lop) { > @@ -161,7 +159,7 @@ static void cio2_fbpt_entry_init_dummy(struct cio2_device *cio2, > > entry[0].first_entry.first_page_offset = 0; > entry[1].second_entry.num_of_pages = CIO2_LOP_ENTRIES * CIO2_MAX_LOPS; > - entry[1].second_entry.last_page_available_bytes = CIO2_PAGE_SIZE - 1; > + entry[1].second_entry.last_page_available_bytes = PAGE_SIZE - 1; > > for (i = 0; i < CIO2_MAX_LOPS; i++) > entry[i].lop_page_addr = cio2->dummy_lop_bus_addr >> PAGE_SHIFT; > @@ -182,25 +180,24 @@ static void cio2_fbpt_entry_init_buf(struct cio2_device *cio2, > entry[0].first_entry.first_page_offset = b->offset; > remaining = length + entry[0].first_entry.first_page_offset; > entry[1].second_entry.num_of_pages = > - DIV_ROUND_UP(remaining, CIO2_PAGE_SIZE); > + DIV_ROUND_UP(remaining, PAGE_SIZE); > /* > * last_page_available_bytes has the offset of the last byte in the > * last page which is still accessible by DMA. DMA cannot access > * beyond this point. Valid range for this is from 0 to 4095. > * 0 indicates 1st byte in the page is DMA accessible. > - * 4095 (CIO2_PAGE_SIZE - 1) means every single byte in the last page > + * 4095 (PAGE_SIZE - 1) means every single byte in the last page > * is available for DMA transfer. > */ > entry[1].second_entry.last_page_available_bytes = > (remaining & ~PAGE_MASK) ? > - (remaining & ~PAGE_MASK) - 1 : > - CIO2_PAGE_SIZE - 1; > + (remaining & ~PAGE_MASK) - 1 : PAGE_SIZE - 1; > /* Fill FBPT */ > remaining = length; > i = 0; > while (remaining > 0) { > entry->lop_page_addr = b->lop_bus_addr[i] >> PAGE_SHIFT; > - remaining -= CIO2_LOP_ENTRIES * CIO2_PAGE_SIZE; > + remaining -= CIO2_LOP_ENTRIES * PAGE_SIZE; > entry++; > i++; > } > @@ -840,7 +837,7 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb) > struct device *dev = &cio2->pci_dev->dev; > struct cio2_buffer *b = > container_of(vb, struct cio2_buffer, vbb.vb2_buf); > - unsigned int pages = DIV_ROUND_UP(vb->planes[0].length, CIO2_PAGE_SIZE); > + unsigned int pages = DIV_ROUND_UP(vb->planes[0].length, PAGE_SIZE); > unsigned int lops = DIV_ROUND_UP(pages + 1, CIO2_LOP_ENTRIES); > struct sg_table *sg; > struct sg_dma_page_iter sg_iter; > @@ -855,7 +852,7 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb) > memset(b->lop, 0, sizeof(b->lop)); > /* Allocate LOP table */ > for (i = 0; i < lops; i++) { > - b->lop[i] = dma_alloc_coherent(dev, CIO2_PAGE_SIZE, > + b->lop[i] = dma_alloc_coherent(dev, PAGE_SIZE, > &b->lop_bus_addr[i], GFP_KERNEL); > if (!b->lop[i]) > goto fail; > @@ -885,8 +882,7 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb) > return 0; > fail: > while (i--) > - dma_free_coherent(dev, CIO2_PAGE_SIZE, > - b->lop[i], b->lop_bus_addr[i]); > + dma_free_coherent(dev, PAGE_SIZE, b->lop[i], b->lop_bus_addr[i]); > return -ENOMEM; > } > > @@ -976,7 +972,7 @@ static void cio2_vb2_buf_cleanup(struct vb2_buffer *vb) > /* Free LOP table */ > for (i = 0; i < CIO2_MAX_LOPS; i++) { > if (b->lop[i]) > - dma_free_coherent(&cio2->pci_dev->dev, CIO2_PAGE_SIZE, > + dma_free_coherent(&cio2->pci_dev->dev, PAGE_SIZE, > b->lop[i], b->lop_bus_addr[i]); > } > } > diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h > index a64a829acc34..549b08f88f0c 100644 > --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h > +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h > @@ -392,7 +392,6 @@ struct cio2_device { > sizeof(struct cio2_fbpt_entry)) > > #define CIO2_FBPT_SUBENTRY_UNIT 4 > -#define CIO2_PAGE_SIZE 4096 > > /* cio2 fbpt first_entry ctrl status */ > #define CIO2_FBPT_CTRL_VALID BIT(0) -- Regards, Laurent Pinchart