| Message ID | 5cc2f8f81f4c7d1ae693d87980353c725f9a11d3.1533637111.git.mchehab+samsung@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: vsp1_dl: add a description for cmdpool field | expand |
Hi Mauro, Thank you for the patch, On 07/08/18 11:18, Mauro Carvalho Chehab wrote: > Gets rid of this build warning: > drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or member 'cmdpool' not described in 'vsp1_dl_manager' > > Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command pools") > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > --- > drivers/media/platform/vsp1/vsp1_dl.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c > index 9255b5ee2cb8..af60d95ec4f8 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -211,6 +211,7 @@ struct vsp1_dl_list { > * @queued: list queued to the hardware (written to the DL registers) > * @pending: list waiting to be queued to the hardware > * @pool: body pool for the display list bodies > + * @cmdpool: Display List commands pool Unfortunately this isn't quite right... > * @autofld_cmds: command pool to support auto-fld interlaced mode This ^ was the original documentation line, but it got missed in a rename. Sorry about that. The pool is now more 'generic' so the line probably should mention the auto-fld directly, so your line is worded appropriately enough, We probably just# need to remove the autofld_cmds line. With that line removed: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> -- Kieran > */ > struct vsp1_dl_manager { >
On 07/08/18 11:36, Kieran Bingham wrote: > Hi Mauro, > > Thank you for the patch, > > On 07/08/18 11:18, Mauro Carvalho Chehab wrote: >> Gets rid of this build warning: >> drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or member 'cmdpool' not described in 'vsp1_dl_manager' >> >> Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command pools") >> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> >> --- >> drivers/media/platform/vsp1/vsp1_dl.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c >> index 9255b5ee2cb8..af60d95ec4f8 100644 >> --- a/drivers/media/platform/vsp1/vsp1_dl.c >> +++ b/drivers/media/platform/vsp1/vsp1_dl.c >> @@ -211,6 +211,7 @@ struct vsp1_dl_list { >> * @queued: list queued to the hardware (written to the DL registers) >> * @pending: list waiting to be queued to the hardware >> * @pool: body pool for the display list bodies >> + * @cmdpool: Display List commands pool > > Unfortunately this isn't quite right... > > >> * @autofld_cmds: command pool to support auto-fld interlaced mode > > This ^ was the original documentation line, but it got missed in a > rename. Sorry about that. > > The pool is now more 'generic' so the line probably should mention the Ahem, clearly I meant "shouldn't" mention. > auto-fld directly, so your line is worded appropriately enough, We > probably just# need to remove the autofld_cmds line. > > > With that line removed: > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > -- > Kieran > > > >> */ >> struct vsp1_dl_manager { >> >
Hello, On Tuesday, 7 August 2018 13:36:59 EEST Kieran Bingham wrote: > On 07/08/18 11:18, Mauro Carvalho Chehab wrote: > > Gets rid of this build warning: > > drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or > > member 'cmdpool' not described in 'vsp1_dl_manager'> > > Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command > > pools") Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > > --- > > > > drivers/media/platform/vsp1/vsp1_dl.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > > b/drivers/media/platform/vsp1/vsp1_dl.c index 9255b5ee2cb8..af60d95ec4f8 > > 100644 > > --- a/drivers/media/platform/vsp1/vsp1_dl.c > > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > > @@ -211,6 +211,7 @@ struct vsp1_dl_list { > > * @queued: list queued to the hardware (written to the DL registers) > > * @pending: list waiting to be queued to the hardware > > * @pool: body pool for the display list bodies > > + * @cmdpool: Display List commands pool > > Unfortunately this isn't quite right... > > > * @autofld_cmds: command pool to support auto-fld interlaced mode > > This ^ was the original documentation line, but it got missed in a > rename. Sorry about that. > > The pool is now more 'generic' so the line probably should mention the > auto-fld directly, so your line is worded appropriately enough, We > probably just# need to remove the autofld_cmds line. And I also wouldn't capitalize display list: * @cmdpool: display list commands pool or possibly a bit more descriptive: * @cmdpool: commands pool for extended display list Kieran, which version do you like best ? > > With that line removed: > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > -- > Kieran > > > */ > > > > struct vsp1_dl_manager {
On 07/08/18 17:17, Laurent Pinchart wrote: > Hello, > > On Tuesday, 7 August 2018 13:36:59 EEST Kieran Bingham wrote: >> On 07/08/18 11:18, Mauro Carvalho Chehab wrote: >>> Gets rid of this build warning: >>> drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or >>> member 'cmdpool' not described in 'vsp1_dl_manager'> >>> Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command >>> pools") Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> >>> --- >>> >>> drivers/media/platform/vsp1/vsp1_dl.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c >>> b/drivers/media/platform/vsp1/vsp1_dl.c index 9255b5ee2cb8..af60d95ec4f8 >>> 100644 >>> --- a/drivers/media/platform/vsp1/vsp1_dl.c >>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c >>> @@ -211,6 +211,7 @@ struct vsp1_dl_list { >>> * @queued: list queued to the hardware (written to the DL registers) >>> * @pending: list waiting to be queued to the hardware >>> * @pool: body pool for the display list bodies >>> + * @cmdpool: Display List commands pool >> >> Unfortunately this isn't quite right... >> >>> * @autofld_cmds: command pool to support auto-fld interlaced mode >> >> This ^ was the original documentation line, but it got missed in a >> rename. Sorry about that. >> >> The pool is now more 'generic' so the line probably should mention the >> auto-fld directly, so your line is worded appropriately enough, We >> probably just# need to remove the autofld_cmds line. > > And I also wouldn't capitalize display list: > > * @cmdpool: display list commands pool > > or possibly a bit more descriptive: > > * @cmdpool: commands pool for extended display list > > Kieran, which version do you like best ? If there's a respin anyway, then * @cmdpool: commands pool for extended display list is certainly better, because this commands are specific to the extended portion of the display list. -- Regards Kieran > >> >> With that line removed: >> >> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> >> -- >> Kieran >> >>> */ >>> >>> struct vsp1_dl_manager { > >
Hi, On Tuesday, 7 August 2018 19:19:41 EEST Kieran Bingham wrote: > On 07/08/18 17:17, Laurent Pinchart wrote: > > On Tuesday, 7 August 2018 13:36:59 EEST Kieran Bingham wrote: > >> On 07/08/18 11:18, Mauro Carvalho Chehab wrote: > >>> Gets rid of this build warning: > >>> drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter > >>> or member 'cmdpool' not described in 'vsp1_dl_manager'> > >>> > >>> Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command > >>> pools") Signed-off-by: Mauro Carvalho Chehab > >>> <mchehab+samsung@kernel.org> > >>> --- > >>> > >>> drivers/media/platform/vsp1/vsp1_dl.c | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > >>> b/drivers/media/platform/vsp1/vsp1_dl.c index 9255b5ee2cb8..af60d95ec4f8 > >>> 100644 > >>> --- a/drivers/media/platform/vsp1/vsp1_dl.c > >>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c > >>> @@ -211,6 +211,7 @@ struct vsp1_dl_list { > >>> * @queued: list queued to the hardware (written to the DL registers) > >>> * @pending: list waiting to be queued to the hardware > >>> * @pool: body pool for the display list bodies > >>> + * @cmdpool: Display List commands pool > >> > >> Unfortunately this isn't quite right... > >> > >>> * @autofld_cmds: command pool to support auto-fld interlaced mode > >> > >> This ^ was the original documentation line, but it got missed in a > >> rename. Sorry about that. > >> > >> The pool is now more 'generic' so the line probably should mention the > >> auto-fld directly, so your line is worded appropriately enough, We > >> probably just# need to remove the autofld_cmds line. > > > > And I also wouldn't capitalize display list: > > > > * @cmdpool: display list commands pool > > > > or possibly a bit more descriptive: > > > > * @cmdpool: commands pool for extended display list > > > > Kieran, which version do you like best ? > > If there's a respin anyway, then > > * @cmdpool: commands pool for extended display list > > is certainly better, because this commands are specific to the extended > portion of the display list. I like that better as well. Mauro, could you please use that for v2 ? > >> With that line removed: > >> > >> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > >> > >>> */ > >>> > >>> struct vsp1_dl_manager {
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c index 9255b5ee2cb8..af60d95ec4f8 100644 --- a/drivers/media/platform/vsp1/vsp1_dl.c +++ b/drivers/media/platform/vsp1/vsp1_dl.c @@ -211,6 +211,7 @@ struct vsp1_dl_list { * @queued: list queued to the hardware (written to the DL registers) * @pending: list waiting to be queued to the hardware * @pool: body pool for the display list bodies + * @cmdpool: Display List commands pool * @autofld_cmds: command pool to support auto-fld interlaced mode */ struct vsp1_dl_manager {
Gets rid of this build warning: drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or member 'cmdpool' not described in 'vsp1_dl_manager' Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command pools") Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> --- drivers/media/platform/vsp1/vsp1_dl.c | 1 + 1 file changed, 1 insertion(+)