Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool
On Fri, Jul 14, 2017 at 3:09 PM, Dan Carpenterwrote: > On Fri, Jul 14, 2017 at 03:55:26PM +0300, Dan Carpenter wrote: >> I don't agree with it as a static analysis dev... > > What I mean is if it's a macro that returns -ENODEV or a function that > returns -ENODEV, they should both be treated the same. The other > warnings this check prints are quite clever. I think this is what gcc tries to do, and it should work normally, but it fails when using ccache. I know I had cases like that, not entirely sure if this is one of them. Maybe it just means I should give up on using ccache in preprocessor mode. Arnd
[PATCH 5/6] v4l: vsp1: Convert LUT to use a fragment pool
Adapt the LUT to allocate a fragment pool for passing the table updates to hardware. Two bodies are pre-allocated in the pool to manage a userspace update before the hardware has taken a previous set of tables. Signed-off-by: Kieran Bingham--- drivers/media/platform/vsp1/vsp1_lut.c | 23 +++ drivers/media/platform/vsp1/vsp1_lut.h | 1 + 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_lut.c b/drivers/media/platform/vsp1/vsp1_lut.c index c67cc60db0db..57482e057e54 100644 --- a/drivers/media/platform/vsp1/vsp1_lut.c +++ b/drivers/media/platform/vsp1/vsp1_lut.c @@ -23,6 +23,8 @@ #define LUT_MIN_SIZE 4U #define LUT_MAX_SIZE 8190U +#define LUT_SIZE 256 + /* - * Device Access */ @@ -44,11 +46,11 @@ static int lut_set_table(struct vsp1_lut *lut, struct v4l2_ctrl *ctrl) struct vsp1_dl_body *dlb; unsigned int i; - dlb = vsp1_dl_fragment_alloc(lut->entity.vsp1, 256); + dlb = vsp1_dl_fragment_get(lut->pool); if (!dlb) return -ENOMEM; - for (i = 0; i < 256; ++i) + for (i = 0; i < LUT_SIZE; ++i) vsp1_dl_fragment_write(dlb, VI6_LUT_TABLE + 4 * i, ctrl->p_new.p_u32[i]); @@ -56,7 +58,7 @@ static int lut_set_table(struct vsp1_lut *lut, struct v4l2_ctrl *ctrl) swap(lut->lut, dlb); spin_unlock_irq(>lock); - vsp1_dl_fragment_free(dlb); + vsp1_dl_fragment_put(dlb); return 0; } @@ -87,7 +89,7 @@ static const struct v4l2_ctrl_config lut_table_control = { .max = 0x00ff, .step = 1, .def = 0, - .dims = { 256}, + .dims = { LUT_SIZE }, }; /* - @@ -217,8 +219,16 @@ static void lut_configure(struct vsp1_entity *entity, } } +static void lut_destroy(struct vsp1_entity *entity) +{ + struct vsp1_lut *lut = to_lut(>subdev); + + vsp1_dl_fragment_pool_free(lut->pool); +} + static const struct vsp1_entity_operations lut_entity_ops = { .configure = lut_configure, + .destroy = lut_destroy, }; /* - @@ -244,6 +254,11 @@ struct vsp1_lut *vsp1_lut_create(struct vsp1_device *vsp1) if (ret < 0) return ERR_PTR(ret); + /* Allocate a fragment pool */ + lut->pool = vsp1_dl_fragment_pool_alloc(vsp1, 2, LUT_SIZE, 0); + if (!lut->pool) + return ERR_PTR(-ENOMEM); + /* Initialize the control handler. */ v4l2_ctrl_handler_init(>ctrls, 1); v4l2_ctrl_new_custom(>ctrls, _table_control, NULL); diff --git a/drivers/media/platform/vsp1/vsp1_lut.h b/drivers/media/platform/vsp1/vsp1_lut.h index f8c4e8f0a79d..538563d57454 100644 --- a/drivers/media/platform/vsp1/vsp1_lut.h +++ b/drivers/media/platform/vsp1/vsp1_lut.h @@ -33,6 +33,7 @@ struct vsp1_lut { spinlock_t lock; struct vsp1_dl_body *lut; + struct vsp1_dl_fragment_pool *pool; }; static inline struct vsp1_lut *to_lut(struct v4l2_subdev *subdev) -- git-series 0.9.1
[PATCH 4/6] v4l: vsp1: Convert CLU to use a fragment pool
Adapt the CLU to allocate a fragment pool for passing the table updates to hardware. Two bodies are pre-allocated in the pool to manage a userspace update before the hardware has taken a previous set of tables. Signed-off-by: Kieran Bingham--- drivers/media/platform/vsp1/vsp1_clu.c | 18 -- drivers/media/platform/vsp1/vsp1_clu.h | 1 + 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_clu.c b/drivers/media/platform/vsp1/vsp1_clu.c index f2fb26e5ab4e..6079c6465435 100644 --- a/drivers/media/platform/vsp1/vsp1_clu.c +++ b/drivers/media/platform/vsp1/vsp1_clu.c @@ -47,7 +47,7 @@ static int clu_set_table(struct vsp1_clu *clu, struct v4l2_ctrl *ctrl) struct vsp1_dl_body *dlb; unsigned int i; - dlb = vsp1_dl_fragment_alloc(clu->entity.vsp1, 1 + 17 * 17 * 17); + dlb = vsp1_dl_fragment_get(clu->pool); if (!dlb) return -ENOMEM; @@ -59,7 +59,7 @@ static int clu_set_table(struct vsp1_clu *clu, struct v4l2_ctrl *ctrl) swap(clu->clu, dlb); spin_unlock_irq(>lock); - vsp1_dl_fragment_free(dlb); + vsp1_dl_fragment_put(dlb); return 0; } @@ -261,8 +261,16 @@ static void clu_configure(struct vsp1_entity *entity, } } +static void clu_destroy(struct vsp1_entity *entity) +{ + struct vsp1_clu *clu = to_clu(>subdev); + + vsp1_dl_fragment_pool_free(clu->pool); +} + static const struct vsp1_entity_operations clu_entity_ops = { .configure = clu_configure, + .destroy = clu_destroy, }; /* - @@ -288,6 +296,12 @@ struct vsp1_clu *vsp1_clu_create(struct vsp1_device *vsp1) if (ret < 0) return ERR_PTR(ret); + /* Allocate a fragment pool */ + clu->pool = vsp1_dl_fragment_pool_alloc(clu->entity.vsp1, 2, + 1 + 17 * 17 * 17, 0); + if (!clu->pool) + return ERR_PTR(-ENOMEM); + /* Initialize the control handler. */ v4l2_ctrl_handler_init(>ctrls, 2); v4l2_ctrl_new_custom(>ctrls, _table_control, NULL); diff --git a/drivers/media/platform/vsp1/vsp1_clu.h b/drivers/media/platform/vsp1/vsp1_clu.h index 036e0a2f1a42..601ffb558e30 100644 --- a/drivers/media/platform/vsp1/vsp1_clu.h +++ b/drivers/media/platform/vsp1/vsp1_clu.h @@ -36,6 +36,7 @@ struct vsp1_clu { spinlock_t lock; unsigned int mode; struct vsp1_dl_body *clu; + struct vsp1_dl_fragment_pool *pool; }; static inline struct vsp1_clu *to_clu(struct v4l2_subdev *subdev) -- git-series 0.9.1
[PATCH 0/6] vsp1: TLB optimisation
Each display list currently allocates an area of DMA memory to store register settings for the VSP1 to process. Each of these allocations adds pressure to the IPMMU TLB entries. We can reduce the pressure by pre-allocating larger areas and dividing the area across multiple bodies represented as a pool. Patch 1 adds protection to ensure that the display list body does not overflow and will allow us to reduce the size of the body allocations in the future (it has already helped me catch an overflow during the development of this series, so I thought it was a worth while addition) Patch 2 implements the fragment pool object and provides function helpers to interact with the pool Patches 3 to 6 convert the existing allocations to use the new fragment pool. These are separated for clarity, but I have no objections to squashing those into a single commit if it is preferred. This series has been tested and based on top of Laurent's recent ES2.0 patch set at git://linuxtv.org/pinchartl/media.git drm/next/h3-es2/merged Kieran Bingham (6): v4l: vsp1: Protect fragments against overflow v4l: vsp1: Provide a fragment pool v4l: vsp1: Convert display lists to use new fragment pool v4l: vsp1: Convert CLU to use a fragment pool v4l: vsp1: Convert LUT to use a fragment pool v4l: vsp1: Remove old fragment management drivers/media/platform/vsp1/vsp1_clu.c | 18 +- drivers/media/platform/vsp1/vsp1_clu.h | 1 +- drivers/media/platform/vsp1/vsp1_dl.c | 294 +- drivers/media/platform/vsp1/vsp1_dl.h | 8 +- drivers/media/platform/vsp1/vsp1_lut.c | 23 +- drivers/media/platform/vsp1/vsp1_lut.h | 1 +- 6 files changed, 199 insertions(+), 146 deletions(-) base-commit: 5fee73960b9a0ceb0ccf746dfeb06bdb07199670 -- git-series 0.9.1
[PATCH 1/6] v4l: vsp1: Protect fragments against overflow
The fragment write function relies on the code never asking it to write more than the entries available in the list. Currently with each list body containing 256 entries, this is fine, but we can reduce this number greatly saving memory. In preparation of this - add a level of protection to catch any buffer overflows Signed-off-by: Kieran Bingham--- drivers/media/platform/vsp1/vsp1_dl.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 8b5cbb6b7a70..1311e7cf2733 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -60,6 +60,7 @@ struct vsp1_dl_body { size_t size; unsigned int num_entries; + unsigned int max_entries; }; /** @@ -138,6 +139,7 @@ static int vsp1_dl_body_init(struct vsp1_device *vsp1, dlb->vsp1 = vsp1; dlb->size = size; + dlb->max_entries = num_entries; dlb->entries = dma_alloc_wc(vsp1->bus_master, dlb->size, >dma, GFP_KERNEL); @@ -220,6 +222,11 @@ void vsp1_dl_fragment_free(struct vsp1_dl_body *dlb) */ void vsp1_dl_fragment_write(struct vsp1_dl_body *dlb, u32 reg, u32 data) { + if (unlikely(dlb->num_entries >= dlb->max_entries)) { + WARN_ONCE(true, "DLB size exceeded"); + return; + } + dlb->entries[dlb->num_entries].addr = reg; dlb->entries[dlb->num_entries].data = data; dlb->num_entries++; -- git-series 0.9.1
[PATCH 6/6] v4l: vsp1: Remove old fragment management
Fragments are no longer 'freed' in interrupt context, but instead released back to their respective pools. This allows us to remove the garbage collector in the DLM. Signed-off-by: Kieran Bingham--- drivers/media/platform/vsp1/vsp1_dl.c | 151 ++- 1 file changed, 14 insertions(+), 137 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 95f2303d37b9..07349fc94372 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -146,9 +146,6 @@ struct vsp1_dl_manager { struct vsp1_dl_list *pending; struct vsp1_dl_fragment_pool *pool; - - struct work_struct gc_work; - struct list_head gc_fragments; }; /* - @@ -252,90 +249,6 @@ void vsp1_dl_fragment_put(struct vsp1_dl_body *dlb) spin_unlock_irqrestore(>pool->lock, flags); } -/* - * Initialize a display list body object and allocate DMA memory for the body - * data. The display list body object is expected to have been initialized to - * 0 when allocated. - */ -static int vsp1_dl_body_init(struct vsp1_device *vsp1, -struct vsp1_dl_body *dlb, unsigned int num_entries, -size_t extra_size) -{ - size_t size = num_entries * sizeof(*dlb->entries) + extra_size; - - dlb->vsp1 = vsp1; - dlb->size = size; - dlb->max_entries = num_entries; - - dlb->entries = dma_alloc_wc(vsp1->bus_master, dlb->size, >dma, - GFP_KERNEL); - if (!dlb->entries) - return -ENOMEM; - - return 0; -} - -/* - * Cleanup a display list body and free allocated DMA memory allocated. - */ -static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb) -{ - dma_free_wc(dlb->vsp1->bus_master, dlb->size, dlb->entries, dlb->dma); -} - -/** - * vsp1_dl_fragment_alloc - Allocate a display list fragment - * @vsp1: The VSP1 device - * @num_entries: The maximum number of entries that the fragment can contain - * - * Allocate a display list fragment with enough memory to contain the requested - * number of entries. - * - * Return a pointer to a fragment on success or NULL if memory can't be - * allocated. - */ -struct vsp1_dl_body *vsp1_dl_fragment_alloc(struct vsp1_device *vsp1, - unsigned int num_entries) -{ - struct vsp1_dl_body *dlb; - int ret; - - dlb = kzalloc(sizeof(*dlb), GFP_KERNEL); - if (!dlb) - return NULL; - - ret = vsp1_dl_body_init(vsp1, dlb, num_entries, 0); - if (ret < 0) { - kfree(dlb); - return NULL; - } - - return dlb; -} - -/** - * vsp1_dl_fragment_free - Free a display list fragment - * @dlb: The fragment - * - * Free the given display list fragment and the associated DMA memory. - * - * Fragments must only be freed explicitly if they are not added to a display - * list, as the display list will take ownership of them and free them - * otherwise. Manual free typically happens at cleanup time for fragments that - * have been allocated but not used. - * - * Passing a NULL pointer to this function is safe, in that case no operation - * will be performed. - */ -void vsp1_dl_fragment_free(struct vsp1_dl_body *dlb) -{ - if (!dlb) - return; - - vsp1_dl_body_cleanup(dlb); - kfree(dlb); -} - /** * vsp1_dl_fragment_write - Write a register to a display list fragment * @dlb: The fragment @@ -392,10 +305,21 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm, return dl; } +static void vsp1_dl_list_fragments_free(struct vsp1_dl_list *dl) +{ + struct vsp1_dl_body *dlb, *tmp; + + list_for_each_entry_safe(dlb, tmp, >fragments, list) { + list_del(>list); + vsp1_dl_fragment_put(dlb); + } +} + static void vsp1_dl_list_free(struct vsp1_dl_list *dl) { vsp1_dl_fragment_put(dl->body0); - list_splice_init(>fragments, >dlm->gc_fragments); + vsp1_dl_list_fragments_free(dl); + kfree(dl); } @@ -449,17 +373,9 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl) dl->has_chain = false; - /* -* We can't free fragments here as DMA memory can only be freed in -* interruptible context. Move all fragments to the display list -* manager's list of fragments to be freed, they will be -* garbage-collected by the work queue. -*/ - if (!list_empty(>fragments)) { - list_splice_init(>fragments, >dlm->gc_fragments); - schedule_work(>dlm->gc_work); - } + vsp1_dl_list_fragments_free(dl); + /* body0 is reused */ dl->body0->num_entries = 0; list_add_tail(>list, >dlm->free); @@ -827,40 +743,6 @@ void
[PATCH 2/6] v4l: vsp1: Provide a fragment pool
Each display list allocates a body to store register values in a dma accessible buffer from a dma_alloc_wc() allocation. Each of these results in an entry in the TLB, and a large number of display list allocations adds pressure to this resource. Reduce TLB pressure on the IPMMUs by allocating multiple display list bodies in a single allocation, and providing these to the display list through a 'fragment pool'. A pool can be allocated by the display list manager or entities which require their own body allocations. Signed-off-by: Kieran Bingham--- drivers/media/platform/vsp1/vsp1_dl.c | 124 +++- drivers/media/platform/vsp1/vsp1_dl.h | 8 ++- 2 files changed, 132 insertions(+) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 1311e7cf2733..8b1118c2e8f5 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -45,6 +45,7 @@ struct vsp1_dl_entry { /** * struct vsp1_dl_body - Display list body * @list: entry in the display list list of bodies + * @pool: entry in the pool list for free/used dlbs * @vsp1: the VSP1 device * @entries: array of entries * @dma: DMA address of the entries @@ -53,6 +54,9 @@ struct vsp1_dl_entry { */ struct vsp1_dl_body { struct list_head list; + struct list_head free; + + struct vsp1_dl_fragment_pool *pool; struct vsp1_device *vsp1; struct vsp1_dl_entry *entries; @@ -64,6 +68,29 @@ struct vsp1_dl_body { }; /** + * struct vsp1_dl_fragment_pool - display list body/fragment pool + * @dma: DMA address of the entries + * @size: size of the full DMA memory pool in bytes + * @mem: CPU memory pointer for the pool + * @bodies: Array of DLB structures for the pool + * @free: List of free DLB entries + * @used: List of active DLB entries + */ +struct vsp1_dl_fragment_pool { + /* DMA allocation */ + dma_addr_t dma; + size_t size; + void *mem; + + /* Body management */ + struct vsp1_dl_body *bodies; + struct list_head free; + spinlock_t lock; + + struct vsp1_device *vsp1; +}; + +/** * struct vsp1_dl_list - Display list * @list: entry in the display list manager lists * @dlm: the display list manager @@ -118,6 +145,8 @@ struct vsp1_dl_manager { struct vsp1_dl_list *queued; struct vsp1_dl_list *pending; + struct vsp1_dl_fragment_pool *pool; + struct work_struct gc_work; struct list_head gc_fragments; }; @@ -127,6 +156,101 @@ struct vsp1_dl_manager { */ /* + * Fragment pool's reduce the pressure on the iommu TLB by allocating a single + * large area of DMA memory and allocating it as a pool of fragment bodies + */ +struct vsp1_dl_fragment_pool * +vsp1_dl_fragment_pool_alloc(struct vsp1_device *vsp1, unsigned int qty, + unsigned int num_entries, size_t extra_size) +{ + struct vsp1_dl_fragment_pool *pool; + size_t dlb_size; + unsigned int i; + + pool = kzalloc(sizeof(*pool), GFP_KERNEL); + if (!pool) + return NULL; + + pool->vsp1 = vsp1; + + dlb_size = num_entries * sizeof(struct vsp1_dl_entry) + extra_size; + pool->size = dlb_size * qty; + + pool->bodies = kcalloc(qty, sizeof(*pool->bodies), GFP_KERNEL); + if (!pool->bodies) { + kfree(pool); + return NULL; + } + + pool->mem = dma_alloc_wc(vsp1->bus_master, pool->size, >dma, + GFP_KERNEL); + if (!pool->mem) { + kfree(pool->bodies); + kfree(pool); + return NULL; + } + + spin_lock_init(>lock); + INIT_LIST_HEAD(>free); + + for (i = 0; i < qty; ++i) { + struct vsp1_dl_body *dlb = >bodies[i]; + + dlb->pool = pool; + dlb->max_entries = num_entries; + dlb->entries = pool->mem + i * dlb_size; + + list_add_tail(>free, >free); + } + + return pool; +} + +void vsp1_dl_fragment_pool_free(struct vsp1_dl_fragment_pool *pool) +{ + if (!pool) + return; + + if (pool->mem) + dma_free_wc(pool->vsp1->bus_master, pool->size, pool->mem, + pool->dma); + + kfree(pool->bodies); + kfree(pool); +} + +struct vsp1_dl_body *vsp1_dl_fragment_get(struct vsp1_dl_fragment_pool *pool) +{ + struct vsp1_dl_body *dlb = NULL; + unsigned long flags; + + spin_lock_irqsave(>lock, flags); + + if (!list_empty(>free)) { + dlb = list_first_entry(>free, struct vsp1_dl_body, free); + list_del(>free); + } + + spin_unlock_irqrestore(>lock, flags); + + return dlb; +} + +void vsp1_dl_fragment_put(struct vsp1_dl_body *dlb) +{ + unsigned long flags; + + if (!dlb) + return; + +
[PATCH 3/6] v4l: vsp1: Convert display lists to use new fragment pool
Adapt the dl->body0 object to use an object from the fragment pool. This greatly reduces the pressure on the TLB for IPMMU use cases, as all of the lists use a single allocation for the main body Signed-off-by: Kieran Bingham--- drivers/media/platform/vsp1/vsp1_dl.c | 68 +++- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 8b1118c2e8f5..95f2303d37b9 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -107,7 +107,7 @@ struct vsp1_dl_list { struct vsp1_dl_header *header; dma_addr_t dma; - struct vsp1_dl_body body0; + struct vsp1_dl_body *body0; struct list_head fragments; bool has_chain; @@ -198,6 +198,8 @@ vsp1_dl_fragment_pool_alloc(struct vsp1_device *vsp1, unsigned int qty, dlb->pool = pool; dlb->max_entries = num_entries; + + dlb->dma = pool->dma + i * dlb_size; dlb->entries = pool->mem + i * dlb_size; list_add_tail(>free, >free); @@ -360,11 +362,10 @@ void vsp1_dl_fragment_write(struct vsp1_dl_body *dlb, u32 reg, u32 data) * Display List Transaction Management */ -static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm) +static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm, + struct vsp1_dl_fragment_pool *pool) { struct vsp1_dl_list *dl; - size_t header_size; - int ret; dl = kzalloc(sizeof(*dl), GFP_KERNEL); if (!dl) @@ -373,32 +374,19 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm) INIT_LIST_HEAD(>fragments); dl->dlm = dlm; - /* -* Initialize the display list body and allocate DMA memory for the body -* and the optional header. Both are allocated together to avoid memory -* fragmentation, with the header located right after the body in -* memory. -*/ - header_size = dlm->mode == VSP1_DL_MODE_HEADER - ? ALIGN(sizeof(struct vsp1_dl_header), 8) - : 0; - - ret = vsp1_dl_body_init(dlm->vsp1, >body0, VSP1_DL_NUM_ENTRIES, - header_size); - if (ret < 0) { - kfree(dl); + /* Retrieve a body from our DLM body pool */ + dl->body0 = vsp1_dl_fragment_get(pool); + if (!dl->body0) return NULL; - } - if (dlm->mode == VSP1_DL_MODE_HEADER) { size_t header_offset = VSP1_DL_NUM_ENTRIES -* sizeof(*dl->body0.entries); +* sizeof(*dl->body0->entries); - dl->header = ((void *)dl->body0.entries) + header_offset; - dl->dma = dl->body0.dma + header_offset; + dl->header = ((void *)dl->body0->entries) + header_offset; + dl->dma = dl->body0->dma + header_offset; memset(dl->header, 0, sizeof(*dl->header)); - dl->header->lists[0].addr = dl->body0.dma; + dl->header->lists[0].addr = dl->body0->dma; } return dl; @@ -406,7 +394,7 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct vsp1_dl_manager *dlm) static void vsp1_dl_list_free(struct vsp1_dl_list *dl) { - vsp1_dl_body_cleanup(>body0); + vsp1_dl_fragment_put(dl->body0); list_splice_init(>fragments, >dlm->gc_fragments); kfree(dl); } @@ -472,7 +460,7 @@ static void __vsp1_dl_list_put(struct vsp1_dl_list *dl) schedule_work(>dlm->gc_work); } - dl->body0.num_entries = 0; + dl->body0->num_entries = 0; list_add_tail(>list, >dlm->free); } @@ -509,7 +497,7 @@ void vsp1_dl_list_put(struct vsp1_dl_list *dl) */ void vsp1_dl_list_write(struct vsp1_dl_list *dl, u32 reg, u32 data) { - vsp1_dl_fragment_write(>body0, reg, data); + vsp1_dl_fragment_write(dl->body0, reg, data); } /** @@ -581,7 +569,7 @@ static void vsp1_dl_list_fill_header(struct vsp1_dl_list *dl, bool is_last) * list was allocated. */ - hdr->num_bytes = dl->body0.num_entries + hdr->num_bytes = dl->body0->num_entries * sizeof(*dl->header->lists); list_for_each_entry(dlb, >fragments, list) { @@ -654,9 +642,9 @@ static void vsp1_dl_list_hw_enqueue(struct vsp1_dl_list *dl) * bit will be cleared by the hardware when the display list * processing starts. */ - vsp1_write(vsp1, VI6_DL_HDR_ADDR(0), dl->body0.dma); + vsp1_write(vsp1, VI6_DL_HDR_ADDR(0), dl->body0->dma); vsp1_write(vsp1, VI6_DL_BODY_SIZE, VI6_DL_BODY_SIZE_UPD | -
Re: [PATCH] ata: sata_rcar: add gen[23] fallback compatibility strings
On Tue, Jul 11, 2017 at 01:44:20PM +0200, Simon Horman wrote: > Add fallback compatibility string for R-Car Gen 2 and 3. > > In the case of Renesas R-Car hardware we know that there are generations of > SoCs, e.g. Gen 1 and 2. But beyond that its not clear what the relationship > between IP blocks might be. For example, I believe that r8a7790 is older > than r8a7791 but that doesn't imply that the latter is a descendant of the > former or vice versa. > > We can, however, by examining the documentation and behaviour of the > hardware at run-time observe that the current driver implementation appears > to be compatible with the IP blocks on SoCs within a given generation. > > For the above reasons and convenience when enabling new SoCs a > per-generation fallback compatibility string scheme being adopted for > drivers for Renesas SoCs. > > Signed-off-by: Simon Horman> --- > Based on libata/for-next > --- > Documentation/devicetree/bindings/ata/sata_rcar.txt | 14 +++--- > drivers/ata/sata_rcar.c | 8 > 2 files changed, 19 insertions(+), 3 deletions(-) Acked-by: Rob Herring
[PATCH v2 5/6] v4l: vsp1: Provide UDS register updates
Provide register definitions required for UDS phase and partition algorithm support. These registers are applicable to Gen3 hardware only. Signed-off-by: Kieran BinghamReviewed-by: Laurent Pinchart --- drivers/media/platform/vsp1/vsp1_regs.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/media/platform/vsp1/vsp1_regs.h b/drivers/media/platform/vsp1/vsp1_regs.h index 58d0bea963a6..26c4ffad2f46 100644 --- a/drivers/media/platform/vsp1/vsp1_regs.h +++ b/drivers/media/platform/vsp1/vsp1_regs.h @@ -396,6 +396,7 @@ #define VI6_UDS_CTRL_NE_RCR(1 << 18) #define VI6_UDS_CTRL_NE_GY (1 << 17) #define VI6_UDS_CTRL_NE_BCB(1 << 16) +#define VI6_UDS_CTRL_AMDSLH(1 << 2) #define VI6_UDS_CTRL_TDIPC (1 << 1) #define VI6_UDS_SCALE 0x2304 @@ -428,11 +429,24 @@ #define VI6_UDS_PASS_BWIDTH_V_MASK (0x7f << 0) #define VI6_UDS_PASS_BWIDTH_V_SHIFT0 +#define VI6_UDS_HPHASE 0x2314 +#define VI6_UDS_HPHASE_HSTP_MASK (0xfff << 16) +#define VI6_UDS_HPHASE_HSTP_SHIFT 16 +#define VI6_UDS_HPHASE_HEDP_MASK (0xfff << 0) +#define VI6_UDS_HPHASE_HEDP_SHIFT 0 + #define VI6_UDS_IPC0x2318 #define VI6_UDS_IPC_FIELD (1 << 27) #define VI6_UDS_IPC_VEDP_MASK (0xfff << 0) #define VI6_UDS_IPC_VEDP_SHIFT 0 +#define VI6_UDS_HSZCLIP0x231c +#define VI6_UDS_HSZCLIP_HCEN (1 << 28) +#define VI6_UDS_HSZCLIP_HCL_OFST_MASK (0xff << 16) +#define VI6_UDS_HSZCLIP_HCL_OFST_SHIFT 16 +#define VI6_UDS_HSZCLIP_HCL_SIZE_MASK (0x1fff << 0) +#define VI6_UDS_HSZCLIP_HCL_SIZE_SHIFT 0 + #define VI6_UDS_CLIP_SIZE 0x2324 #define VI6_UDS_CLIP_SIZE_HSIZE_MASK (0x1fff << 16) #define VI6_UDS_CLIP_SIZE_HSIZE_SHIFT 16 -- git-series 0.9.1
[PATCH v2 4/6] v4l: vsp1: Move partition rectangles to struct and operate directly
As we develop the partition algorithm, we need to store more information per partition to describe the phase and other parameters. To keep this data together, further abstract the existing v4l2_rect into a partition specific structure When generating the partition windows, operate directly on the partition struct rather than copying and duplicating the processed data Signed-off-by: Kieran BinghamReviewed-by: Laurent Pinchart --- drivers/media/platform/vsp1/vsp1_pipe.h | 12 +- drivers/media/platform/vsp1/vsp1_rpf.c | 4 +- drivers/media/platform/vsp1/vsp1_uds.c | 8 ++-- drivers/media/platform/vsp1/vsp1_video.c | 50 + drivers/media/platform/vsp1/vsp1_wpf.c | 14 +++ 5 files changed, 50 insertions(+), 38 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h index f440d0891bea..44506a315b5d 100644 --- a/drivers/media/platform/vsp1/vsp1_pipe.h +++ b/drivers/media/platform/vsp1/vsp1_pipe.h @@ -58,6 +58,14 @@ enum vsp1_pipeline_state { }; /* + * struct vsp1_partition - A description of each partition slice performed by HW + * @dest: The position and dimension of this partition in the destination image + */ +struct vsp1_partition { + struct v4l2_rect dest; +}; + +/* * struct vsp1_pipeline - A VSP1 hardware pipeline * @pipe: the media pipeline * @irqlock: protects the pipeline state @@ -114,8 +122,8 @@ struct vsp1_pipeline { struct vsp1_dl_list *dl; unsigned int partitions; - struct v4l2_rect partition; - struct v4l2_rect *part_table; + struct vsp1_partition *partition; + struct vsp1_partition *part_table; }; void vsp1_pipeline_reset(struct vsp1_pipeline *pipe); diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c b/drivers/media/platform/vsp1/vsp1_rpf.c index 8feddd59cf8d..48b3e89c0e87 100644 --- a/drivers/media/platform/vsp1/vsp1_rpf.c +++ b/drivers/media/platform/vsp1/vsp1_rpf.c @@ -108,9 +108,9 @@ static void rpf_configure(struct vsp1_entity *entity, output = vsp1_entity_get_pad_format(wpf, wpf->config, RWPF_PAD_SINK); - crop.width = pipe->partition.width * input_width + crop.width = pipe->partition->dest.width * input_width / output->width; - crop.left += pipe->partition.left * input_width + crop.left += pipe->partition->dest.left * input_width / output->width; } diff --git a/drivers/media/platform/vsp1/vsp1_uds.c b/drivers/media/platform/vsp1/vsp1_uds.c index 4226403ad235..ea37ff5aee57 100644 --- a/drivers/media/platform/vsp1/vsp1_uds.c +++ b/drivers/media/platform/vsp1/vsp1_uds.c @@ -272,11 +272,13 @@ static void uds_configure(struct vsp1_entity *entity, bool multitap; if (params == VSP1_ENTITY_PARAMS_PARTITION) { - const struct v4l2_rect *clip = >partition; + struct vsp1_partition *partition = pipe->partition; vsp1_uds_write(uds, dl, VI6_UDS_CLIP_SIZE, - (clip->width << VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) | - (clip->height << VI6_UDS_CLIP_SIZE_VSIZE_SHIFT)); + (partition->dest.width + << VI6_UDS_CLIP_SIZE_HSIZE_SHIFT) | + (partition->dest.height + << VI6_UDS_CLIP_SIZE_VSIZE_SHIFT)); return; } diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c index dd94968d0044..42e5608d1ddf 100644 --- a/drivers/media/platform/vsp1/vsp1_video.c +++ b/drivers/media/platform/vsp1/vsp1_video.c @@ -185,17 +185,17 @@ static int __vsp1_video_try_format(struct vsp1_video *video, /** * vsp1_video_partition - Calculate the active partition output window * + * @partition: The active partition data * @div_size: pre-determined maximum partition division size * @index: partition index * - * Returns a v4l2_rect describing the partition window. + * Generates the output partitioning positions. */ -static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe, -unsigned int div_size, -unsigned int index) +static void vsp1_video_partition(struct vsp1_pipeline *pipe, +struct vsp1_partition *partition, +unsigned int div_size, unsigned int index) { const struct v4l2_mbus_framefmt *format; - struct v4l2_rect partition; unsigned int modulus; /* @@ -208,18 +208,19 @@ static struct v4l2_rect vsp1_video_partition(struct
[PATCH v2 3/6] v4l: vsp1: Remove redundant context variables
The vsp1_pipe object context variables for div_size and current_partition allowed state to be maintained through processing the partitions during processing. Now that the partition tables are calculated during stream on, there is no requirement to store these variables in the pipe object. Utilise local variables for the processing as required. Signed-off-by: Kieran BinghamReviewed-by: Laurent Pinchart --- v2: - s/current_partition/partition/ - s/partition_number/partition/ --- drivers/media/platform/vsp1/vsp1_pipe.h | 4 drivers/media/platform/vsp1/vsp1_video.c | 22 +++--- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h index d345cc6d0d69..f440d0891bea 100644 --- a/drivers/media/platform/vsp1/vsp1_pipe.h +++ b/drivers/media/platform/vsp1/vsp1_pipe.h @@ -80,10 +80,8 @@ enum vsp1_pipeline_state { * @uds_input: entity at the input of the UDS, if the UDS is present * @entities: list of entities in the pipeline * @dl: display list associated with the pipeline - * @div_size: The maximum allowed partition size for the pipeline * @partitions: The number of partitions used to process one frame * @partition: The current partition for configuration to process - * @current_partition: The partition number currently being configured * @part_table: The pre-calculated partitions used by the pipeline */ struct vsp1_pipeline { @@ -115,10 +113,8 @@ struct vsp1_pipeline { struct vsp1_dl_list *dl; - unsigned int div_size; unsigned int partitions; struct v4l2_rect partition; - unsigned int current_partition; struct v4l2_rect *part_table; }; diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c index 4155c06e9b42..dd94968d0044 100644 --- a/drivers/media/platform/vsp1/vsp1_video.c +++ b/drivers/media/platform/vsp1/vsp1_video.c @@ -280,7 +280,6 @@ static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe) if (pipe->part_table == NULL) return -ENOMEM; - pipe->div_size = div_size; pipe->partitions = 1; pipe->part_table[0] = vsp1_video_partition(pipe, div_size, 0); return 0; @@ -296,7 +295,6 @@ static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe) } } - pipe->div_size = div_size; pipe->partitions = DIV_ROUND_UP(format->width, div_size); pipe->part_table = kcalloc(pipe->partitions, sizeof(*pipe->part_table), @@ -385,11 +383,12 @@ static void vsp1_video_frame_end(struct vsp1_pipeline *pipe, } static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe, - struct vsp1_dl_list *dl) + struct vsp1_dl_list *dl, + unsigned int partition) { struct vsp1_entity *entity; - pipe->partition = pipe->part_table[pipe->current_partition]; + pipe->partition = pipe->part_table[partition]; list_for_each_entry(entity, >entities, list_pipe) { if (entity->ops->configure) @@ -402,6 +401,7 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe) { struct vsp1_device *vsp1 = pipe->output->entity.vsp1; struct vsp1_entity *entity; + unsigned int partition; if (!pipe->dl) pipe->dl = vsp1_dl_list_get(pipe->output->dlm); @@ -418,20 +418,12 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe) } /* Run the first partition */ - pipe->current_partition = 0; - vsp1_video_pipeline_run_partition(pipe, pipe->dl); + vsp1_video_pipeline_run_partition(pipe, pipe->dl, 0); /* Process consecutive partitions as necessary */ - for (pipe->current_partition = 1; -pipe->current_partition < pipe->partitions; -pipe->current_partition++) { + for (partition = 1; partition < pipe->partitions; partition++) { struct vsp1_dl_list *dl; - /* -* Partition configuration operations will utilise -* the pipe->current_partition variable to determine -* the work they should complete. -*/ dl = vsp1_dl_list_get(pipe->output->dlm); /* @@ -444,7 +436,7 @@ static void vsp1_video_pipeline_run(struct vsp1_pipeline *pipe) break; } - vsp1_video_pipeline_run_partition(pipe, dl); + vsp1_video_pipeline_run_partition(pipe, dl, partition); vsp1_dl_list_add_chain(pipe->dl, dl); } -- git-series 0.9.1
[PATCH v2 6/6] v4l: vsp1: Allow entities to participate in the partition algorithm
The configuration of the pipeline and entities directly affects the inputs required to each entity for the partition algorithm. Thus it makes sense to involve those entities in the decision making process. Extend the entity ops API to provide an optional '.partition' operation. This allows entities that effect the partition window to adapt the window based on their configuration. Entities implementing this operation must return their required input parameters, which will be passed up the pipeline. This creates a process whereby each entity describes what is required to satisfy the required output to its predecessor in the pipeline. Signed-off-by: Kieran Bingham--- v2: - vsp1_partition_rect renamed as vsp1_partition_window - destination vsp1_partition_window made const to prevent change - (currently) unused 'offset' removed. - partition functions made static --- drivers/media/platform/vsp1/vsp1_entity.h | 8 +- drivers/media/platform/vsp1/vsp1_pipe.c | 22 ++- drivers/media/platform/vsp1/vsp1_pipe.h | 35 -- drivers/media/platform/vsp1/vsp1_rpf.c| 31 +-- drivers/media/platform/vsp1/vsp1_sru.c| 30 ++- drivers/media/platform/vsp1/vsp1_uds.c| 39 ++-- drivers/media/platform/vsp1/vsp1_video.c | 30 ++ drivers/media/platform/vsp1/vsp1_wpf.c| 33 ++-- 8 files changed, 188 insertions(+), 40 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h index 11f8363fa6b0..7475b504ca3b 100644 --- a/drivers/media/platform/vsp1/vsp1_entity.h +++ b/drivers/media/platform/vsp1/vsp1_entity.h @@ -21,6 +21,7 @@ struct vsp1_device; struct vsp1_dl_list; struct vsp1_pipeline; +struct vsp1_partition; enum vsp1_entity_type { VSP1_ENTITY_BRS, @@ -82,12 +83,19 @@ struct vsp1_route { * selection rectangles, ...) * @max_width: Return the max supported width of data that the entity can * process in a single operation. + * @partition: Process the partition construction based on this entity's + * configuration. */ struct vsp1_entity_operations { void (*destroy)(struct vsp1_entity *); void (*configure)(struct vsp1_entity *, struct vsp1_pipeline *, struct vsp1_dl_list *, enum vsp1_entity_params); unsigned int (*max_width)(struct vsp1_entity *, struct vsp1_pipeline *); + struct vsp1_partition_window *(*partition)(struct vsp1_entity *, + struct vsp1_pipeline *, + struct vsp1_partition *, + unsigned int, + const struct vsp1_partition_window *); }; struct vsp1_entity { diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c b/drivers/media/platform/vsp1/vsp1_pipe.c index 4f4b732df84b..8e16f901ab80 100644 --- a/drivers/media/platform/vsp1/vsp1_pipe.c +++ b/drivers/media/platform/vsp1/vsp1_pipe.c @@ -383,6 +383,28 @@ void vsp1_pipeline_propagate_alpha(struct vsp1_pipeline *pipe, vsp1_uds_set_alpha(pipe->uds, dl, alpha); } +/* + * Propagate the partition calculations through the pipeline + * + * Work backwards through the pipe, allowing each entity to update the partition + * parameters based on its configuration, and the entity connected to its + * source. Each entity must produce the partition required for the previous + * entity in the pipeline. + */ +void vsp1_pipeline_propagate_partition(struct vsp1_pipeline *pipe, + struct vsp1_partition *partition, + unsigned int index, + struct vsp1_partition_window *rect) +{ + struct vsp1_entity *entity; + + list_for_each_entry_reverse(entity, >entities, list_pipe) { + if (entity->ops->partition) + rect = entity->ops->partition(entity, pipe, partition, + index, rect); + } +} + void vsp1_pipelines_suspend(struct vsp1_device *vsp1) { unsigned long flags; diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h index 44506a315b5d..d5858a308ec0 100644 --- a/drivers/media/platform/vsp1/vsp1_pipe.h +++ b/drivers/media/platform/vsp1/vsp1_pipe.h @@ -58,11 +58,32 @@ enum vsp1_pipeline_state { }; /* + * struct vsp1_partition_window + * + * replicates struct v4l2_rect, but allows us to extend on the needs of each + * partition. + */ +struct vsp1_partition_window { + __s32 left; + __s32 top; + __u32 width; + __u32 height; +}; + +/* * struct vsp1_partition - A description of each partition slice performed by HW - * @dest: The position and dimension of this partition in the destination image + *
[PATCH v2 2/6] v4l: vsp1: Calculate partition sizes at stream start.
Previously the active window and partition sizes for each partition were calculated for each partition every frame. This data is constant and only needs to be calculated once at the start of the stream. Extend the vsp1_pipe object to dynamically store the number of partitions required and pre-calculate the partition sizes into this table. Signed-off-by: Kieran BinghamReviewed-by: Laurent Pinchart --- v2: - Partition table allocated dynamically as required. - loop uses unsigned int - rebase on top of suspend-resume fix commits --- drivers/media/platform/vsp1/vsp1_pipe.h | 3 ++- drivers/media/platform/vsp1/vsp1_video.c | 32 + 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h b/drivers/media/platform/vsp1/vsp1_pipe.h index c5d01a365370..d345cc6d0d69 100644 --- a/drivers/media/platform/vsp1/vsp1_pipe.h +++ b/drivers/media/platform/vsp1/vsp1_pipe.h @@ -82,7 +82,9 @@ enum vsp1_pipeline_state { * @dl: display list associated with the pipeline * @div_size: The maximum allowed partition size for the pipeline * @partitions: The number of partitions used to process one frame + * @partition: The current partition for configuration to process * @current_partition: The partition number currently being configured + * @part_table: The pre-calculated partitions used by the pipeline */ struct vsp1_pipeline { struct media_pipeline pipe; @@ -117,6 +119,7 @@ struct vsp1_pipeline { unsigned int partitions; struct v4l2_rect partition; unsigned int current_partition; + struct v4l2_rect *part_table; }; void vsp1_pipeline_reset(struct vsp1_pipeline *pipe); diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c index 03e139f40dcf..4155c06e9b42 100644 --- a/drivers/media/platform/vsp1/vsp1_video.c +++ b/drivers/media/platform/vsp1/vsp1_video.c @@ -256,12 +256,13 @@ static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe, return partition; } -static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe) +static int vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe) { struct vsp1_device *vsp1 = pipe->output->entity.vsp1; const struct v4l2_mbus_framefmt *format; struct vsp1_entity *entity; unsigned int div_size; + unsigned int i; /* * Partitions are computed on the size before rotation, use the format @@ -274,9 +275,15 @@ static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe) /* Gen2 hardware doesn't require image partitioning. */ if (vsp1->info->gen == 2) { + pipe->part_table = kcalloc(1, sizeof(*pipe->part_table), + GFP_ATOMIC); + if (pipe->part_table == NULL) + return -ENOMEM; + pipe->div_size = div_size; pipe->partitions = 1; - return; + pipe->part_table[0] = vsp1_video_partition(pipe, div_size, 0); + return 0; } list_for_each_entry(entity, >entities, list_pipe) { @@ -291,6 +298,16 @@ static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe) pipe->div_size = div_size; pipe->partitions = DIV_ROUND_UP(format->width, div_size); + + pipe->part_table = kcalloc(pipe->partitions, sizeof(*pipe->part_table), + GFP_ATOMIC); + if (pipe->part_table == NULL) + return -ENOMEM; + + for (i = 0; i < pipe->partitions; i++) + pipe->part_table[i] = vsp1_video_partition(pipe, div_size, i); + + return 0; } /* - * Pipeline Management @@ -372,8 +389,7 @@ static void vsp1_video_pipeline_run_partition(struct vsp1_pipeline *pipe, { struct vsp1_entity *entity; - pipe->partition = vsp1_video_partition(pipe, pipe->div_size, - pipe->current_partition); + pipe->partition = pipe->part_table[pipe->current_partition]; list_for_each_entry(entity, >entities, list_pipe) { if (entity->ops->configure) @@ -801,9 +817,12 @@ static void vsp1_video_buffer_queue(struct vb2_buffer *vb) static int vsp1_video_setup_pipeline(struct vsp1_pipeline *pipe) { struct vsp1_entity *entity; + int ret; /* Determine this pipelines sizes for image partitioning support. */ - vsp1_video_pipeline_setup_partitions(pipe); + ret = vsp1_video_pipeline_setup_partitions(pipe); + if (ret) + return ret; /* Prepare the display list. */ pipe->dl = vsp1_dl_list_get(pipe->output->dlm); @@ -908,6 +927,9 @@ static void
[PATCH v2 0/6] vsp1 partition algorithm improvements
Some updates and initial improvements for the VSP1 partition algorithm that remove redundant processing and variables, reducing the processing done in interrupt context slightly. Patches 1, 2 and 3 clean up the calculation of the partition windows such that they are only calculated once at streamon. Patch 4 improves the allocations with a new vsp1_partition object to track each window state. Patches 5, and 6 then go on to enhance the partition algorithm by allowing each entity to calculate the requirements for it's pipeline predecessor to successfully generate the requested output window. This system allows the entity objects to specify what they need to fulfil the output for the next entity in the pipeline. v2: - Rebased to v4.12-rc1 - Partition tables dynamically allocated - review fixups Kieran Bingham (6): v4l: vsp1: Move vsp1_video_pipeline_setup_partitions() function v4l: vsp1: Calculate partition sizes at stream start. v4l: vsp1: Remove redundant context variables v4l: vsp1: Move partition rectangles to struct and operate directly v4l: vsp1: Provide UDS register updates v4l: vsp1: Allow entities to participate in the partition algorithm drivers/media/platform/vsp1/vsp1_entity.h | 8 +- drivers/media/platform/vsp1/vsp1_pipe.c | 22 +++- drivers/media/platform/vsp1/vsp1_pipe.h | 48 ++- drivers/media/platform/vsp1/vsp1_regs.h | 14 ++- drivers/media/platform/vsp1/vsp1_rpf.c| 31 ++-- drivers/media/platform/vsp1/vsp1_sru.c| 30 - drivers/media/platform/vsp1/vsp1_uds.c| 43 +- drivers/media/platform/vsp1/vsp1_video.c | 163 --- drivers/media/platform/vsp1/vsp1_wpf.c| 33 +++-- 9 files changed, 289 insertions(+), 103 deletions(-) base-commit: 70e60837e669f6fbc8bba30db9a2244d347643bc -- git-series 0.9.1
[PATCH v2 1/6] v4l: vsp1: Move vsp1_video_pipeline_setup_partitions() function
Separate the code change from the function move so that code changes can be clearly identified. This commit has no functional change. The partition algorithm functions will be changed, and vsp1_video_pipeline_setup_partitions() will call vsp1_video_partition(). To prepare for that, move the function without any code change. Signed-off-by: Kieran BinghamReviewed-by: Laurent Pinchart --- drivers/media/platform/vsp1/vsp1_video.c | 73 - 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_video.c b/drivers/media/platform/vsp1/vsp1_video.c index e9f5dcb8fae5..03e139f40dcf 100644 --- a/drivers/media/platform/vsp1/vsp1_video.c +++ b/drivers/media/platform/vsp1/vsp1_video.c @@ -182,43 +182,6 @@ static int __vsp1_video_try_format(struct vsp1_video *video, * VSP1 Partition Algorithm support */ -static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe) -{ - struct vsp1_device *vsp1 = pipe->output->entity.vsp1; - const struct v4l2_mbus_framefmt *format; - struct vsp1_entity *entity; - unsigned int div_size; - - /* -* Partitions are computed on the size before rotation, use the format -* at the WPF sink. -*/ - format = vsp1_entity_get_pad_format(>output->entity, - pipe->output->entity.config, - RWPF_PAD_SINK); - div_size = format->width; - - /* Gen2 hardware doesn't require image partitioning. */ - if (vsp1->info->gen == 2) { - pipe->div_size = div_size; - pipe->partitions = 1; - return; - } - - list_for_each_entry(entity, >entities, list_pipe) { - unsigned int entity_max = VSP1_VIDEO_MAX_WIDTH; - - if (entity->ops->max_width) { - entity_max = entity->ops->max_width(entity, pipe); - if (entity_max) - div_size = min(div_size, entity_max); - } - } - - pipe->div_size = div_size; - pipe->partitions = DIV_ROUND_UP(format->width, div_size); -} - /** * vsp1_video_partition - Calculate the active partition output window * @@ -293,6 +256,42 @@ static struct v4l2_rect vsp1_video_partition(struct vsp1_pipeline *pipe, return partition; } +static void vsp1_video_pipeline_setup_partitions(struct vsp1_pipeline *pipe) +{ + struct vsp1_device *vsp1 = pipe->output->entity.vsp1; + const struct v4l2_mbus_framefmt *format; + struct vsp1_entity *entity; + unsigned int div_size; + + /* +* Partitions are computed on the size before rotation, use the format +* at the WPF sink. +*/ + format = vsp1_entity_get_pad_format(>output->entity, + pipe->output->entity.config, + RWPF_PAD_SINK); + div_size = format->width; + + /* Gen2 hardware doesn't require image partitioning. */ + if (vsp1->info->gen == 2) { + pipe->div_size = div_size; + pipe->partitions = 1; + return; + } + + list_for_each_entry(entity, >entities, list_pipe) { + unsigned int entity_max = VSP1_VIDEO_MAX_WIDTH; + + if (entity->ops->max_width) { + entity_max = entity->ops->max_width(entity, pipe); + if (entity_max) + div_size = min(div_size, entity_max); + } + } + + pipe->div_size = div_size; + pipe->partitions = DIV_ROUND_UP(format->width, div_size); +} /* - * Pipeline Management */ -- git-series 0.9.1
Re: clk: renesas: rcar-gen3: Fix error return code in cpg_sd_clock_recalc_rate
On Thu, Apr 20, 2017 at 02:46:33AM +0900, Yoshihiro Kaneko wrote: > From: Takeshi Kihara> > In .recalc_rate of struct clk_ops, it is desirable to return 0 if an > error occurs, but -EINVAL is returned. This patch fixes it. > > Fixes: 5b1defde7054 ("clk: renesas: cpg-mssr: Extract common R-Car Gen3 > support code") > Signed-off-by: Takeshi Kihara > Signed-off-by: Yoshihiro Kaneko Reviewed-by: Wolfram Sang signature.asc Description: PGP signature
Re: [PATCH] clk: renesas: rcar-gen3: Fix error return code in cpg_sd_clock_recalc_rate
On Thu, Jul 13, 2017 at 11:06:56PM +0200, Wolfram Sang wrote: > From: Takeshi Kihara> > In .recalc_rate of struct clk_ops, it is desirable to return 0 if an > error occurs, but -EINVAL is returned. This patch fixes it. > > Fixes: 5b1defde7054 ("clk: renesas: cpg-mssr: Extract common R-Car Gen3 > support code") > Signed-off-by: Takeshi Kihara > Signed-off-by: Wolfram Sang I just saw that Kaneko-san posted this patch as well: https://patchwork.kernel.org/patch/9688509/ Please pick his patch, I'll add my Rev-by there. signature.asc Description: PGP signature
Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool
On Fri, Jul 14, 2017 at 03:55:26PM +0300, Dan Carpenter wrote: > I don't agree with it as a static analysis dev... What I mean is if it's a macro that returns -ENODEV or a function that returns -ENODEV, they should both be treated the same. The other warnings this check prints are quite clever. regards, dan carpenter
Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool
On Fri, Jul 14, 2017 at 11:36:56AM +0200, Arnd Bergmann wrote: > @@ -1158,7 +1158,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc) >*/ > fmt_src.pad = pad->index; > fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE; > - if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, _src)) { > + ret = v4l2_subdev_call(sensor, pad, get_fmt, NULL, _src); > + if (!ret) { > fmt_info = omap3isp_video_format_info(fmt_src.format.code); > depth_in = fmt_info->width; > } Is the original code buggy? media/platform/omap3isp/ispccdc.c 1156 /* Compute the lane shifter shift value and enable the bridge when the 1157 * input format is a non-BT.656 YUV variant. 1158 */ 1159 fmt_src.pad = pad->index; 1160 fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE; 1161 if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, _src)) { 1162 fmt_info = omap3isp_video_format_info(fmt_src.format.code); 1163 depth_in = fmt_info->width; 1164 } If v4l2_subdev_call() then depth_in is zero. 1165 1166 fmt_info = omap3isp_video_format_info(format->code); 1167 depth_out = fmt_info->width; 1168 shift = depth_in - depth_out; How do we know that this subtraction doesn't set "shift" to a very high positive? 1169 1170 if (ccdc->bt656) 1171 bridge = ISPCTRL_PAR_BRIDGE_DISABLE; 1172 else if (fmt_info->code == MEDIA_BUS_FMT_YUYV8_2X8) 1173 bridge = ISPCTRL_PAR_BRIDGE_LENDIAN; 1174 else if (fmt_info->code == MEDIA_BUS_FMT_UYVY8_2X8) 1175 bridge = ISPCTRL_PAR_BRIDGE_BENDIAN; 1176 else 1177 bridge = ISPCTRL_PAR_BRIDGE_DISABLE; 1178 1179 omap3isp_configure_bridge(isp, ccdc->input, parcfg, shift, bridge); regards, dan carpenter
Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool
On Fri, Jul 14, 2017 at 2:05 PM, Dan Carpenterwrote: > Changing: > > - if (!frob()) { > + if (frob() == 0) { > > is a totally pointless change. They're both bad, because they're doing > success testing instead of failure testing, but probably the second one > is slightly worse. > > This warning seems dumb. I can't imagine it has even a 10% success rate > at finding real bugs. Just disable it. > > Changing the code to propagate error codes, is the right thing of course > so long as it doesn't introduce bugs. It found a two of bugs that I fixed earlier: f0e8faa7a5e8 ("ARM: ux500: fix prcmu_is_cpu_in_wfi() calculation") af15769ffab1 ("scsi: mvsas: fix command_active typo") plus three patches from this series: 1. staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read 2. isdn: isdnloop: suppress a gcc-7 warning (my patch is wrong, as Joe pointed out there is a real bug) 3. drm/vmwgfx: avoid gcc-7 parentheses (here, Linus had a better analysis of the problem, so we should consider that a bug as well) I would estimate around 25% success rate here, which isn't that bad for a new warning. I agree that most of the false positives are really dumb though. Arnd
Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool
Changing: - if (!frob()) { + if (frob() == 0) { is a totally pointless change. They're both bad, because they're doing success testing instead of failure testing, but probably the second one is slightly worse. This warning seems dumb. I can't imagine it has even a 10% success rate at finding real bugs. Just disable it. Changing the code to propagate error codes, is the right thing of course so long as it doesn't introduce bugs. regards, dan carpenter
[PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool
v4l2_subdev_call is a macro returning whatever the callback return type is, usually 'int'. With gcc-7 and ccache, this can lead to many wanings like: media/platform/pxa_camera.c: In function 'pxa_mbus_build_fmts_xlate': media/platform/pxa_camera.c:766:27: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context] while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, )) { media/atomisp/pci/atomisp2/atomisp_cmd.c: In function 'atomisp_s_ae_window': media/atomisp/pci/atomisp2/atomisp_cmd.c:6414:52: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context] if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera, The best workaround I could come up with is to change all the callers that use the return code from v4l2_subdev_call() in an 'if' or 'while' condition. In case of simple 'if' checks, adding a temporary variable is usually ok, and sometimes this can be used to propagate or print an error code, so I do that. For the 'while' loops, I ended up adding an otherwise useless comparison with zero, which unfortunately makes the code a little uglied. Signed-off-by: Arnd Bergmann--- drivers/media/pci/cx18/cx18-ioctl.c | 6 -- drivers/media/pci/saa7146/mxb.c | 5 +++-- drivers/media/platform/atmel/atmel-isc.c | 4 ++-- drivers/media/platform/atmel/atmel-isi.c | 4 ++-- drivers/media/platform/blackfin/bfin_capture.c | 4 ++-- drivers/media/platform/omap3isp/ispccdc.c| 5 +++-- drivers/media/platform/pxa_camera.c | 3 ++- drivers/media/platform/rcar-vin/rcar-core.c | 2 +- drivers/media/platform/rcar-vin/rcar-dma.c | 4 +++- drivers/media/platform/soc_camera/soc_camera.c | 4 ++-- drivers/media/platform/stm32/stm32-dcmi.c| 4 ++-- drivers/media/platform/ti-vpe/cal.c | 6 -- drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 13 +++-- 13 files changed, 37 insertions(+), 27 deletions(-) diff --git a/drivers/media/pci/cx18/cx18-ioctl.c b/drivers/media/pci/cx18/cx18-ioctl.c index 80b902b12a78..1803f28fc501 100644 --- a/drivers/media/pci/cx18/cx18-ioctl.c +++ b/drivers/media/pci/cx18/cx18-ioctl.c @@ -188,6 +188,7 @@ static int cx18_g_fmt_sliced_vbi_cap(struct file *file, void *fh, { struct cx18 *cx = fh2id(fh)->cx; struct v4l2_sliced_vbi_format *vbifmt = >fmt.sliced; + int ret; /* sane, V4L2 spec compliant, defaults */ vbifmt->reserved[0] = 0; @@ -201,8 +202,9 @@ static int cx18_g_fmt_sliced_vbi_cap(struct file *file, void *fh, * digitizer/slicer. Note, cx18_av_vbi() wipes the passed in * fmt->fmt.sliced under valid calling conditions */ - if (v4l2_subdev_call(cx->sd_av, vbi, g_sliced_fmt, >fmt.sliced)) - return -EINVAL; + ret = v4l2_subdev_call(cx->sd_av, vbi, g_sliced_fmt, >fmt.sliced); + if (ret) + return ret; vbifmt->service_set = cx18_get_service_set(vbifmt); return 0; diff --git a/drivers/media/pci/saa7146/mxb.c b/drivers/media/pci/saa7146/mxb.c index 504d78807639..d2d843c38579 100644 --- a/drivers/media/pci/saa7146/mxb.c +++ b/drivers/media/pci/saa7146/mxb.c @@ -525,8 +525,9 @@ static int vidioc_s_input(struct file *file, void *fh, unsigned int input) return err; /* switch video in saa7111a */ - if (saa7111a_call(mxb, video, s_routing, i, SAA7111_FMT_CCIR, 0)) - pr_err("VIDIOC_S_INPUT: could not address saa7111a\n"); + err = saa7111a_call(mxb, video, s_routing, i, SAA7111_FMT_CCIR, 0); + if (err) + pr_err("VIDIOC_S_INPUT: could not address saa7111a: %d\n", err); mxb->cur_audinput = video_audio_connect[input]; /* switch the audio-source only if necessary */ diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c index d6534252cdcd..704b34a0cc00 100644 --- a/drivers/media/platform/atmel/atmel-isc.c +++ b/drivers/media/platform/atmel/atmel-isc.c @@ -1475,8 +1475,8 @@ static int isc_formats_init(struct isc_device *isc) fmt++; } - while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, - NULL, _code)) { + while (v4l2_subdev_call(subdev, pad, enum_mbus_code, + NULL, _code) == 0) { mbus_code.index++; fmt = find_format_by_code(mbus_code.code, ); if (!fmt) diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c index 891fa2505efa..30b7e6f298ed 100644 --- a/drivers/media/platform/atmel/atmel-isi.c +++ b/drivers/media/platform/atmel/atmel-isi.c @@ -1013,8 +1013,8 @@ static int isi_formats_init(struct atmel_isi *isi) .which = V4L2_SUBDEV_FORMAT_ACTIVE, }; - while
Re: [PATCH 4/4] drm/sun4i: make sure we don't have a commit pending
Hi, On Thu, Jul 13, 2017 at 10:41 PM, Maxime Ripardwrote: > In the earlier display engine designs, any register access while a commit > is pending is forbidden. > > One of the symptoms is that reading a register will return another, random, > register value which can lead to register corruptions if we ever do a > read/modify/write cycle. Alternatively, if changes to the backend (layers) are guaranteed to happen while the CRTC is disabled (which seems to be the case after looking at drm_atomic_helper_commit_planes and drm_atomic_helper_commit_tail), we could just turn on register auto-commit all the time and not deal with this. ChenYu > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/sun4i/sun4i_backend.c | 14 ++ > drivers/gpu/drm/sun4i/sun4i_crtc.c| 1 + > 2 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c > b/drivers/gpu/drm/sun4i/sun4i_backend.c > index cf480218daa5..ce1f40f7511e 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c > @@ -67,6 +67,19 @@ static void sun4i_backend_commit(struct sunxi_engine > *engine) > SUN4I_BACKEND_REGBUFFCTL_LOADCTL); > } > > +static int sun4i_backend_commit_poll(struct sunxi_engine *engine) > +{ > + u32 val; > + > + DRM_DEBUG_DRIVER("Polling for the commit to end\n"); > + > + return regmap_read_poll_timeout(engine->regs, > + SUN4I_BACKEND_REGBUFFCTL_REG, > + val, > + !(val & > SUN4I_BACKEND_REGBUFFCTL_LOADCTL), > + 100, 5); > +} > + > void sun4i_backend_layer_enable(struct sun4i_backend *backend, > int layer, bool enable) > { > @@ -330,6 +343,7 @@ static int sun4i_backend_of_get_id(struct device_node > *node) > > static const struct sunxi_engine_ops sun4i_backend_engine_ops = { > .commit = sun4i_backend_commit, > + .commit_poll= sun4i_backend_commit_poll, > .layers_init= sun4i_layers_init, > .apply_color_correction = > sun4i_backend_apply_color_correction, > .disable_color_correction = > sun4i_backend_disable_color_correction, > diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c > b/drivers/gpu/drm/sun4i/sun4i_crtc.c > index 2eba1d8639d8..31550493fa1d 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c > +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c > @@ -34,6 +34,7 @@ static void sun4i_crtc_atomic_begin(struct drm_crtc *crtc, > struct drm_crtc_state *old_state) > { > struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc); > + struct sunxi_engine *engine = scrtc->engine; > struct drm_device *dev = crtc->dev; > unsigned long flags; > > -- > git-series 0.9.1
[PATCH] arm64: dts: renesas: ulcb: sound clock-frequency needs descending order
From: Vladimir BarinovCorrect order of sound clock frequencies for ULCB boards used with r8a7795 and r8a7796 SoCs. These sounds clock frequencies are used as the ADG clock (output clocks for audio module) initial setting and sound codec's initial system clock which needs the maximum clock frequency. Thus, descending order is required. Fixes: 9f22774c214ada7b ("arm64: dts: ulcb: add 12288000 for sound ADG") Signed-off-by: Vladimir Barinov Reviewed-by: Geert Uytterhoeven [simon: rewrote changelog] Signed-off-by: Simon Horman --- arch/arm64/boot/dts/renesas/ulcb.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi b/arch/arm64/boot/dts/renesas/ulcb.dtsi index b5c6ee07d7f9..d1a3f3b7a0ab 100644 --- a/arch/arm64/boot/dts/renesas/ulcb.dtsi +++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi @@ -281,7 +281,7 @@ /* audio_clkout0/1/2/3 */ #clock-cells = <1>; - clock-frequency = <11289600 12288000>; + clock-frequency = <12288000 11289600>; status = "okay"; -- 2.1.4
[GIT PULL] Second Round of Renesas ARM Based SoC Fixes for v4.13
Hi Olof, Hi Kevin, Hi Arnd, Please consider these second round of Renesas ARM based SoC fixes for v4.13. This pull request is based on the previous round of such requests, tagged as renesas-fixes-for-v4.13, which I have already sent a pull-request for. That pull request provides the same fix as this pull one but for the Salvator rather than ULCB boards. On a personal note, I will be on holidays from the 17th - 26th July. I apologise in advance for any inconvenience that may cause. The following changes since commit 5e2feac330953fe75197aecb20c781400e2bf606: arm64: renesas: salvator-common: sound clock-frequency needs descending order (2017-06-19 11:18:33 +0200) are available in the git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-fixes2-for-v4.13 for you to fetch changes up to 2752660a37aed65b1e00fd4563d9f152eefb8200: arm64: dts: renesas: ulcb: sound clock-frequency needs descending order (2017-07-10 10:28:07 +0200) Second Round of Renesas ARM Based SoC Fixes for v4.13 Correct order of sound clock frequencies for ULCB boards used by r8a7795 and r8a7796 SoCs. These sounds clock frequencies are used as the ADG clock (output clocks for audio module) initial setting and sound codec's initial system clock which needs the maximum clock frequency. Thus, descending order is required. Vladimir Barinov (1): arm64: dts: renesas: ulcb: sound clock-frequency needs descending order arch/arm64/boot/dts/renesas/ulcb.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: [PATCH v2 08/14] v4l: vsp1: Add support for new VSP2-BS, VSP2-DL and VSP2-D instances
On 14/07/17 00:31, Laurent Pinchart wrote: > Hi Kieran, > > On Thursday 13 Jul 2017 18:49:19 Kieran Bingham wrote: >> On 26/06/17 19:12, Laurent Pinchart wrote: >>> New Gen3 SoCs come with two new VSP2 variants names VSP2-BS and VSP2-DL, >>> as well as a new VSP2-D variant on V3M and V3H SoCs. Add new entries for >>> them in the VSP device info table. >>> >>> Signed-off-by: Laurent Pinchart >>>>> >> Code in the patch looks OK - but I can't see where the difference between >> the horizontal widths are supported between VSPD H3/VC >> >> I see this in the datasheet: (32.1.1.6 in this particular part) >> >> Direct connection to display module >> — Supporting 4096 pixels in horizontal direction [R-Car H3/R-Car M3-W/ R-Car >> M3-N] >> — Supporting 2048 pixels in horizontal direction [R-Car V3M/R-Car V3H/R-Car >> D3/R-Car E3] >> >> Do we have this information encoded anywhere? or are they just talking about >> maximum performance capability there? > > No, we don't. It's a limit that we should have. I think we should fix that in > a separate patch, as the 4096 pixels limit isn't implemented either. I'm not so worried about these limits (unless they cause the hardware to hang), but I think they should be encoded somewhere, and I would certainly count that as a separate patch. Of course (excluding pipelines using BRU/BRS) the partition algorithm could provide the capability to support image processing beyond limitations of the pipeline maximum size... But this can't cater for throughput limitations of bandwidth so there will be a limit to how big an image we really want to support ... Non realtime processing of large megapixel images might be relevant though - but still not a use case to worry about here. >> Also some features that are implied as supported aren't mentioned - but >> that's not a blocker to adding in the initial devices at all. >> >> Therefore: >> >> Reviewed-by: Kieran Bingham >> >>> --- >>> >>> drivers/media/platform/vsp1/vsp1_drv.c | 24 >>> drivers/media/platform/vsp1/vsp1_regs.h | 15 +-- >>> 2 files changed, 37 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c >>> b/drivers/media/platform/vsp1/vsp1_drv.c index 6a9aeb71aedf..c4f2ac61f7d2 >>> 100644 >>> --- a/drivers/media/platform/vsp1/vsp1_drv.c >>> +++ b/drivers/media/platform/vsp1/vsp1_drv.c >>> @@ -710,6 +710,14 @@ static const struct vsp1_device_info >>> vsp1_device_infos[] = {> >>> .num_bru_inputs = 5, >>> .uapi = true, >>> }, { >>> + .version = VI6_IP_VERSION_MODEL_VSPBS_GEN3, >>> + .model = "VSP2-BS", >>> + .gen = 3, >>> + .features = VSP1_HAS_BRS, >> >> 32.1.1.5 implies: >> | VSP1_HAS_WPF_VFLIP >> >> But Figure 32.5 implies that it doesn't ... > > The figures only tell whether the full combination of rotation and H/V flip > is > available. I think you're right, I'll add VSP1_HAS_WPF_VFLIP. > >> Figure 32.5 also implies that | VSP1_HAS_CLU is there too on both RPF0, and >> RPF1 > > Note that CLUT != CLU. I know it's confusing :-) Oh ! :-S ... /me goes back to the datasheet ... > >>> + .rpf_count = 2, >>> + .wpf_count = 1, >>> + .uapi = true, >>> + }, { >>> .version = VI6_IP_VERSION_MODEL_VSPD_GEN3, >>> .model = "VSP2-D", >>> .gen = 3, >>> @@ -717,6 +725,22 @@ static const struct vsp1_device_info >>> vsp1_device_infos[] = {> >>> .rpf_count = 5, >>> .wpf_count = 2, >>> .num_bru_inputs = 5, >>> + }, { >>> + .version = VI6_IP_VERSION_MODEL_VSPD_V3, >>> + .model = "VSP2-D", >>> + .gen = 3, >>> + .features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF, >>> + .rpf_count = 5, >>> + .wpf_count = 1, >>> + .num_bru_inputs = 5, >>> + }, { >>> + .version = VI6_IP_VERSION_MODEL_VSPDL_GEN3, >>> + .model = "VSP2-DL", >>> + .gen = 3, >>> + .features = VSP1_HAS_BRS | VSP1_HAS_BRU | VSP1_HAS_LIF, >> >> Hrm. 32.1.1.7 says: >> — Vertical flipping in case of output to memory. >> So thats some sort of a conditional : | VSP1_HAS_WPF_VFLIP >> >> So looking at this and the settings of the existing models, I guess it looks >> like we don't support flip if we have an LIF output (as that would then be >> unsupported) > > On Gen3 vertical flipping seems to always be supported, unlike on Gen2 where > VSPD is specifically documented as not supporting vertical flipping. We could > add the WFLIP on all VSP2-D* instances. This would create a corresponding > control, which wouldn't do much harm as the VSPD instances on Gen3 are not > exposed to userspace, but that would waste a bit of memory for no good > purpose > (beside correctness I suppose). I wonder if that's worth it, what do you > think > ? If
Re: [PATCH v2] ARM: dts: iwg20m: Add MMCIF0 support
On Thu, Jul 13, 2017 at 06:31:33PM +0200, Geert Uytterhoeven wrote: > On Thu, Jul 13, 2017 at 5:39 PM, Chris Paterson >wrote: > > Define the iwg20m board dependent part of the MMCIF0 device node. > > > > Signed-off-by: Chris Paterson > > Reviewed-by: Geert Uytterhoeven Thanks, applied for v4.14.
Re: [PATCH] arm64: dts: renesas: salvator-xs: Add VC6 clock generator
On Thu, Jul 13, 2017 at 01:19:12PM +0200, Geert Uytterhoeven wrote: > On Thu, Jul 13, 2017 at 1:06 PM, Laurent Pinchart >wrote: > > The VC6 is an I2C-controlled programmable clock generator, used on the > > board to provide a display dot clock. Add it to DT. > > > > Signed-off-by: Laurent Pinchart > > Reviewed-by: Geert Uytterhoeven Thanks, applied for v4.14.
Re: [PATCH 2/2] arm64: dts: r8a7795: salvator-xs: Connect DU dot clocks 0 and 3
On Thu, Jul 13, 2017 at 01:20:16PM +0200, Geert Uytterhoeven wrote: > On Thu, Jul 13, 2017 at 1:09 PM, Laurent Pinchart >wrote: > > The DU dot clocks 0 and 3 are provided by the programmable VC6 clock > > generator. Connect them to the clock source node. > > > > Signed-off-by: Laurent Pinchart > > Reviewed-by: Geert Uytterhoeven Thanks, applied for v4.14.
Re: [PATCH 2/2] arm64: dts: renesas: r8a7796: Add resets property to VSP devices
On Fri, Jul 14, 2017 at 09:20:31AM +0200, Simon Horman wrote: > On Thu, Jul 13, 2017 at 01:17:10PM +0200, Geert Uytterhoeven wrote: > > On Thu, Jul 13, 2017 at 12:58 PM, Laurent Pinchart > >wrote: > > > The VSP nodes are missing the resets property. Add it. > > > > > > Fixes: 5a89c826745f ("arm64: dts: renesas: r8a7796: Add VSP instances") > > > Signed-off-by: Laurent Pinchart > > > > > > > Reviewed-by: Geert Uytterhoeven > > Thanks, I have squashed this into the commit referenced above. The result is as follows: From: Laurent Pinchart Subject: arm64: dts: renesas: r8a7796: Add VSP instances The r8a7796 has 5 VSP instances. Signed-off-by: Laurent Pinchart Reviewed-by: Geert Uytterhoeven Signed-off-by: Simon Horman --- arch/arm64/boot/dts/renesas/r8a7796.dtsi | 55 1 file changed, 55 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi index b301554ff424..9ef1729a800c 100644 --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi @@ -1570,6 +1570,17 @@ resets = < 615>; }; + vspb: vsp@fe96 { + compatible = "renesas,vsp2"; + reg = <0 0xfe96 0 0x8000>; + interrupts = ; + clocks = < CPG_MOD 626>; + power-domains = < R8A7796_PD_A3VC>; + resets = < 626>; + + renesas,fcp = <>; + }; + fcpvb0: fcp@fe96f000 { compatible = "renesas,fcpv"; reg = <0 0xfe96f000 0 0x200>; @@ -1578,6 +1589,17 @@ resets = < 607>; }; + vspi0: vsp@fe9a { + compatible = "renesas,vsp2"; + reg = <0 0xfe9a 0 0x8000>; + interrupts = ; + clocks = < CPG_MOD 631>; + power-domains = < R8A7796_PD_A3VC>; + resets = < 631>; + + renesas,fcp = <>; + }; + fcpvi0: fcp@fe9af000 { compatible = "renesas,fcpv"; reg = <0 0xfe9af000 0 0x200>; @@ -1586,6 +1608,17 @@ resets = < 611>; }; + vspd0: vsp@fea2 { + compatible = "renesas,vsp2"; + reg = <0 0xfea2 0 0x4000>; + interrupts = ; + clocks = < CPG_MOD 623>; + power-domains = < R8A7796_PD_ALWAYS_ON>; + resets = < 623>; + + renesas,fcp = <>; + }; + fcpvd0: fcp@fea27000 { compatible = "renesas,fcpv"; reg = <0 0xfea27000 0 0x200>; @@ -1594,6 +1627,17 @@ resets = < 603>; }; + vspd1: vsp@fea28000 { + compatible = "renesas,vsp2"; + reg = <0 0xfea28000 0 0x4000>; + interrupts = ; + clocks = < CPG_MOD 622>; + power-domains = < R8A7796_PD_ALWAYS_ON>; + resets = < 622>; + + renesas,fcp = <>; + }; + fcpvd1: fcp@fea2f000 { compatible = "renesas,fcpv"; reg = <0 0xfea2f000 0 0x200>; @@ -1602,6 +1646,17 @@ resets = < 602>; }; + vspd2: vsp@fea3 { + compatible = "renesas,vsp2"; + reg = <0 0xfea3 0 0x4000>; + interrupts = ; + clocks = < CPG_MOD 621>; + power-domains = < R8A7796_PD_ALWAYS_ON>; + resets = < 621>; + + renesas,fcp = <>; + }; + fcpvd2: fcp@fea37000 { compatible = "renesas,fcpv"; reg = <0 0xfea37000 0 0x200>; -- 2.1.4
Re: [PATCH 1/2] arm64: dts: renesas: r8a7796: Add resets property to FCP devices
On Thu, Jul 13, 2017 at 01:16:19PM +0200, Geert Uytterhoeven wrote: > On Thu, Jul 13, 2017 at 12:58 PM, Laurent Pinchart >wrote: > > The FCP nodes are missing the resets property. Add it. > > > > Fixes: 7153c69fb8e1 ("arm64: dts: renesas: r8a7796: Add FCPF and FCPV > > instances") > > Signed-off-by: Laurent Pinchart > > Reviewed-by: Geert Uytterhoeven Thanks, I have squashed this into the commit referenced above. The result is as follows: From: Laurent Pinchart Subject: arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances The FCPs handle the interface between various IP cores and memory. Add the instances related to the FDPs and VSP2s. Signed-off-by: Laurent Pinchart Reviewed-by: Geert Uytterhoeven Signed-off-by: Simon Horman --- arch/arm64/boot/dts/renesas/r8a7796.dtsi | 48 1 file changed, 48 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi index 367da78ad4fc..b301554ff424 100644 --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi @@ -1562,6 +1562,54 @@ /* placeholder */ }; + fcpf0: fcp@fe95 { + compatible = "renesas,fcpf"; + reg = <0 0xfe95 0 0x200>; + clocks = < CPG_MOD 615>; + power-domains = < R8A7796_PD_A3VC>; + resets = < 615>; + }; + + fcpvb0: fcp@fe96f000 { + compatible = "renesas,fcpv"; + reg = <0 0xfe96f000 0 0x200>; + clocks = < CPG_MOD 607>; + power-domains = < R8A7796_PD_A3VC>; + resets = < 607>; + }; + + fcpvi0: fcp@fe9af000 { + compatible = "renesas,fcpv"; + reg = <0 0xfe9af000 0 0x200>; + clocks = < CPG_MOD 611>; + power-domains = < R8A7796_PD_A3VC>; + resets = < 611>; + }; + + fcpvd0: fcp@fea27000 { + compatible = "renesas,fcpv"; + reg = <0 0xfea27000 0 0x200>; + clocks = < CPG_MOD 603>; + power-domains = < R8A7796_PD_ALWAYS_ON>; + resets = < 603>; + }; + + fcpvd1: fcp@fea2f000 { + compatible = "renesas,fcpv"; + reg = <0 0xfea2f000 0 0x200>; + clocks = < CPG_MOD 602>; + power-domains = < R8A7796_PD_ALWAYS_ON>; + resets = < 602>; + }; + + fcpvd2: fcp@fea37000 { + compatible = "renesas,fcpv"; + reg = <0 0xfea37000 0 0x200>; + clocks = < CPG_MOD 601>; + power-domains = < R8A7796_PD_ALWAYS_ON>; + resets = < 601>; + }; + du: display@feb0 { /* placeholder */ -- 2.1.4
Re: [PATCH 2/2] arm64: dts: renesas: r8a7796: Add resets property to VSP devices
On Thu, Jul 13, 2017 at 01:17:10PM +0200, Geert Uytterhoeven wrote: > On Thu, Jul 13, 2017 at 12:58 PM, Laurent Pinchart >wrote: > > The VSP nodes are missing the resets property. Add it. > > > > Fixes: 5a89c826745f ("arm64: dts: renesas: r8a7796: Add VSP instances") > > Signed-off-by: Laurent Pinchart > > Reviewed-by: Geert Uytterhoeven Thanks, I have squashed this into the commit referenced above.