From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: linux-media@vger.kernel.org, Yong Zhi <yong.zhi@intel.com>,
Sakari Ailus <sakari.ailus@linux.intel.com>,
Bingbu Cao <bingbu.cao@intel.com>,
Tian Shu Qiu <tian.shu.qiu@intel.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>
Subject: Re: [PATCH v2 03/10] media: ipu2-cio2: Replace custom definition with PAGE_SIZE
Date: Fri, 9 Oct 2020 04:04:56 +0300 [thread overview]
Message-ID: <20201009010456.GF12857@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20200817160734.12402-3-andriy.shevchenko@linux.intel.com>
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 <andriy.shevchenko@linux.intel.com>
> ---
> 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
next prev parent reply other threads:[~2020-10-09 1:05 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-17 16:07 [PATCH v2 01/10] media: ipu3-cio2: Simplify cleanup code Andy Shevchenko
2020-08-17 16:07 ` [PATCH v2 02/10] media: ipu3-cio2: Introduce CIO2_LOP_ENTRIES constant Andy Shevchenko
2020-10-09 1:00 ` Laurent Pinchart
2020-10-09 10:11 ` Andy Shevchenko
2020-08-17 16:07 ` [PATCH v2 03/10] media: ipu2-cio2: Replace custom definition with PAGE_SIZE Andy Shevchenko
2020-10-09 1:04 ` Laurent Pinchart [this message]
2020-10-09 11:42 ` Andy Shevchenko
2020-08-17 16:07 ` [PATCH v2 04/10] media: ipu3-cio2: Use macros from pfn.h Andy Shevchenko
2020-08-17 16:07 ` [PATCH v2 05/10] media: ipu3-cio2: Replace infinite loop by one with clear exit condition Andy Shevchenko
2020-08-17 16:07 ` [PATCH v2 06/10] media: ipu3-cio2: Use readl_poll_timeout() helper Andy Shevchenko
2020-10-09 1:14 ` Laurent Pinchart
2020-08-17 16:07 ` [PATCH v2 07/10] media: ipu3-cio2: Get rid of pci_set_master() duplication Andy Shevchenko
2020-10-09 1:09 ` Laurent Pinchart
2020-08-17 16:07 ` [PATCH v2 08/10] media: ipu3-cio2: Drop bogus check and error message Andy Shevchenko
2020-10-09 1:18 ` Laurent Pinchart
2020-10-09 10:17 ` Andy Shevchenko
2020-08-17 16:07 ` [PATCH v2 09/10] media: ipu3-cio2: Drop useless assignments Andy Shevchenko
2020-10-09 1:19 ` Laurent Pinchart
2020-08-17 16:07 ` [PATCH v2 10/10] media: ipu3-cio2: Update Copyright year and fix indentation issues Andy Shevchenko
2020-10-09 1:15 ` Laurent Pinchart
2020-10-09 0:57 ` [PATCH v2 01/10] media: ipu3-cio2: Simplify cleanup code Laurent Pinchart
2020-10-09 10:26 ` Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201009010456.GF12857@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bingbu.cao@intel.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=tian.shu.qiu@intel.com \
--cc=yong.zhi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.