Hi Kieran,
Thank you for the patch.
On Thursday, 8 March 2018 02:05:24 EEST Kieran Bingham wrote:
> Throughout the codebase, the term 'fragment' is used to represent a
> display list body. This term duplicates the 'body' which is already in
> use.
>
> The datasheet references these objects as a body, therefore replace all
> mentions of a fragment with a body, along with the corresponding
> pluralised terms.
I like this, the code seems less confusing to me this way. Please see below
for a few minor comments.
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
>
> ---
> v7
> - Clean up the formatting of the vsp1_dl_list_add_body()
>
> drivers/media/platform/vsp1/vsp1_clu.c | 10 +-
> drivers/media/platform/vsp1/vsp1_dl.c | 109 ++++++++++++--------------
> drivers/media/platform/vsp1/vsp1_dl.h | 13 +--
> drivers/media/platform/vsp1/vsp1_lut.c | 8 +-
> 4 files changed, 69 insertions(+), 71 deletions(-)
[snip]
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c index 0b86ed01e85d..caed441f5f0c
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
[snip]
> @@ -157,17 +157,16 @@ static void vsp1_dl_body_cleanup(struct
> vsp1_dl_body *dlb) }
>
> /**
> - * vsp1_dl_fragment_alloc - Allocate a display list fragment
> + * vsp1_dl_body_alloc - Allocate a display list body
> * @vsp1: The VSP1 device
> - * @num_entries: The maximum number of entries that the fragment can
> contain
> + * @num_entries: The maximum number of entries that the body can contain
> *
> - * Allocate a display list fragment with enough memory to contain the
> requested
> + * Allocate a display list body 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.
> + * Return a pointer to a body on success or NULL if memory can't be
> allocated.
> */
> -struct vsp1_dl_body *vsp1_dl_fragment_alloc(struct vsp1_device *vsp1,
> +struct vsp1_dl_body *vsp1_dl_body_alloc(struct vsp1_device *vsp1,
> unsigned int num_entries)
The indentation of the second line now looks wrong.
[snip]
> @@ -379,33 +378,33 @@ 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(&dl->body0, reg, data);
> + vsp1_dl_body_write(&dl->body0, reg, data);
> }
>
> /**
> - * vsp1_dl_list_add_fragment - Add a fragment to the display list
> + * vsp1_dl_list_add_body - Add a body to the display list
> * @dl: The display list
> - * @dlb: The fragment
> + * @dlb: The body
> *
> - * Add a display list body as a fragment to a display list. Registers
> contained
> - * in fragments are processed after registers contained in the main display
> - * list, in the order in which fragments are added.
> + * Add a display list body as a body to a display list. Registers contained
"body as a body" sounds strange. How about just "Add a display list body to
the display list." ?
> + * in bodies are processed after registers contained in the main display
> list,
> + * in the order in which bodies are added.
> *
> - * Adding a fragment to a display list passes ownership of the fragment to
> the
> - * list. The caller must not touch the fragment after this call, and
> must not
> - * free it explicitly with vsp1_dl_fragment_free().
> + * Adding a body to a display list passes ownership of the body to the
> list. The
> + * caller must not touch the body after this call, and must not free it
> + * explicitly with vsp1_dl_body_free().
> *
> - * Fragments are only usable for display lists in header mode. Attempt to
> - * add a fragment to a header-less display list will return an error.
> + * Additional bodies are only usable for display lists in header mode.
> + * Attempting to add a body to a header-less display list will return an
> error.
> */
[snip]
With those two small issues fixed,
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
--
Regards,
Laurent Pinchart
