Re: [PATCH 02/16] media: omap3isp: allow it to build with COMPILE_TEST
Hi Mauro, I love your patch! Yet something to improve: [auto build test ERROR on linuxtv-media/master] [also build test ERROR on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-drivers-under-drivers-media-to-build-with-COMPILE_TEST/20180406-164215 base: git://linuxtv.org/media_tree.git master config: openrisc-allyesconfig (attached as .config) compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental) reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=openrisc All errors (new ones prefixed by >>): In file included from drivers/media/platform/omap3isp/isp.c:78:0: drivers/media/platform/omap3isp/isp.h:130:16: error: field 'hw' has incomplete type struct clk_hw hw; ^~ In file included from include/asm-generic/bug.h:5:0, from ./arch/openrisc/include/generated/asm/bug.h:1, from include/linux/bug.h:5, from include/linux/mmdebug.h:5, from include/linux/mm.h:9, from arch/openrisc/include/asm/cacheflush.h:21, from drivers/media/platform/omap3isp/isp.c:45: drivers/media/platform/omap3isp/isp.c: In function 'isp_xclk_prepare': include/linux/kernel.h:938:32: error: dereferencing pointer to incomplete type 'struct clk_hw' BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ ^~ include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert' bool __cond = !(condition);\ ^ include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~ include/linux/kernel.h:938:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ ^~~~ include/linux/kernel.h:938:20: note: in expansion of macro '__same_type' BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ ^~~ drivers/media/platform/omap3isp/isp.c:167:26: note: in expansion of macro 'container_of' #define to_isp_xclk(_hw) container_of(_hw, struct isp_xclk, hw) ^~~~ drivers/media/platform/omap3isp/isp.c:187:26: note: in expansion of macro 'to_isp_xclk' struct isp_xclk *xclk = to_isp_xclk(hw); ^~~ drivers/media/platform/omap3isp/isp.c: At top level: drivers/media/platform/omap3isp/isp.c:282:21: error: variable 'isp_xclk_ops' has initializer but incomplete type static const struct clk_ops isp_xclk_ops = { ^~~ >> drivers/media/platform/omap3isp/isp.c:283:2: error: unknown field 'prepare' >> specified in initializer .prepare = isp_xclk_prepare, ^ drivers/media/platform/omap3isp/isp.c:283:13: warning: excess elements in struct initializer .prepare = isp_xclk_prepare, ^~~~ drivers/media/platform/omap3isp/isp.c:283:13: note: (near initialization for 'isp_xclk_ops') >> drivers/media/platform/omap3isp/isp.c:284:2: error: unknown field >> 'unprepare' specified in initializer .unprepare = isp_xclk_unprepare, ^ drivers/media/platform/omap3isp/isp.c:284:15: warning: excess elements in struct initializer .unprepare = isp_xclk_unprepare, ^~ drivers/media/platform/omap3isp/isp.c:284:15: note: (near initialization for 'isp_xclk_ops') >> drivers/media/platform/omap3isp/isp.c:285:2: error: unknown field 'enable' >> specified in initializer .enable = isp_xclk_enable, ^ drivers/media/platform/omap3isp/isp.c:285:12: warning: excess elements in struct initializer .enable = isp_xclk_enable, ^~~ drivers/media/platform/omap3isp/isp.c:285:12: note: (near initialization for 'isp_xclk_ops') >> drivers/media/platform/omap3isp/isp.c:286:2: error: unknown field 'disable' >> specified in initializer .disable = isp_xclk_disable, ^ drivers/media/platform/omap3isp/isp.c:286:13: warning: excess elements in struct initializer .disable = isp_xclk_disable, ^~~~ drivers/media/platform/omap3isp/isp.c:286:13: note: (near in
cron job: media_tree daily build: OK
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Sat Apr 7 05:00:16 CEST 2018 media-tree git hash:17dec0a949153d9ac00760ba2f5b78cb583e995f media_build git hash: 541653bb52fcf7c34b8b83a4c8cc66b09c68ac37 v4l-utils git hash: 47d43b130dc6e9e0edc900759fb37649208371e4 gcc version:i686-linux-gcc (GCC) 7.3.0 sparse version: v0.5.2-rc1 smatch version: v0.5.0-4419-g3b5bf5c9 host hardware: x86_64 host os:4.14.0-3-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.101-i686: OK linux-3.2.101-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.56-i686: OK linux-3.16.56-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.102-i686: OK linux-3.18.102-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.51-i686: OK linux-4.1.51-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.109-i686: OK linux-4.4.109-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.91-i686: OK linux-4.9.91-x86_64: OK linux-4.14.31-i686: OK linux-4.14.31-x86_64: OK linux-4.15.14-i686: OK linux-4.15.14-x86_64: OK linux-4.16-i686: OK linux-4.16-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v7 0/8] vsp1: TLB optimisation and DL caching
Hi Kieran, I've finished reviewing the series. For your convenience, I've rebased it on top of the BRU/BRS dynamic allocation patches, and pushed the result to git://linuxtv.org/pinchartl/media.git v4l2/vsp1/tlb-optimise (Please note it has been compile-tested only) I have also taken the liberty to incorporate both my review comments and my Reviewed-by line for the patches that have received a conditional Reviewed-by, that is patches 1/8, 5/8 and 7/8. Patches 3/8, 4/8, 6/8 and 8/8 have open questions so I haven't touched them. Please don't despair, v8 might well be the last version we will need :-) On Thursday, 8 March 2018 02:05:23 EEST Kieran Bingham wrote: > 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. > > With this reconfiguration of bodies, we can adapt the configuration code to > separate out constant hardware configuration and cache it for re-use. > > The patches provided in this series can be found at: > git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git > tags/vsp1/tlb-optimise/v7 > > I hope that this series is at a stage where it could be integrated now. It > has had some thorough testing and is already integrated in both > renesas-drivers and renesas-bsp. (except for the minor changes in v7 that > is...) > > Please note that checkpatch complains on patch 6/8 in this series: > > v7-0006-media-vsp1-Refactor-display-list-configure-operations.patch > > -- WARNING: function definition argument 'struct > vsp1_entity *' should also have an identifier name #290: FILE: > drivers/media/platform/vsp1/vsp1_entity.h:82: > + void (*configure_stream)(struct vsp1_entity *, struct vsp1_pipeline > *, > > However - this complaint is regarding pre-existing code. I have only renamed > the function pointers. I do also disagree with checkpatch here - as there > is no need to provide an identifier name, and it does not improve > readability in this instance to state: > ...(vsp1_entity *entity, struct vsp1_pipeline *pipe) > > Thus - I have ignored these warnings. > > > Changelog: > -- > > v7: > - Rebased on to linux-media/master (v4.16-rc4) > - Clean up the formatting of the vsp1_dl_list_add_body() > - Fix formatting and white space > - s/prepare/configure_stream/ > - s/configure/configure_frame/ > > v6: > - Rebased on to linux-media/master (v4.16-rc1) > - Removed DRM/UIF (DISCOM/ColorKey) updates > > v5: > - Rebased on to renesas-drivers-2018-01-09-v4.15-rc7 to fix conflicts >with DRM and UIF updates on VSP1 driver > > v4: > - Rebased to v4.14 > * v4l: vsp1: Use reference counting for bodies >- Fix up reference handling comments > > * v4l: vsp1: Provide a body pool >- Provide comment explaining extra allocation on body pool > highlighting area for optimisation later. > > * v4l: vsp1: Refactor display list configure operations >- Fix up comment to describe yuv_mode caching rather than format > > * vsp1: Adapt entities to configure into a body >- Rename vsp1_dl_list_get_body() to vsp1_dl_list_get_body0() > > * v4l: vsp1: Move video configuration to a cached dlb >- Adjust pipe configured flag to be reset on resume rather than suspend >- rename dl_child, dl_next > > Testing: > > The VSP unit tests have been run on this patch set with the following > results: > > --- Test loop 1 --- > - vsp-unit-test-.sh > Test Conditions: > Platform Renesas Salvator-X 2nd version board based on r8a7795 > ES2.0+ Kernel release4.16.0-rc4-arm64-renesas-01067-g397eb3811ec0 > convert /usr/bin/convert > compare /usr/bin/compare > killall /usr/bin/killall > raw2rgbpnm/usr/bin/raw2rgbpnm > stress/usr/bin/stress > yavta /usr/bin/yavta > - vsp-unit-test-0001.sh > Testing WPF packing in RGB332: pass > Testing WPF packing in ARGB555: pass > Testing WPF packing in XRGB555: pass > Testing WPF packing in RGB565: pass > Testing WPF packing in BGR24: pass > Testing WPF packing in RGB24: pass > Testing WPF packing in ABGR32: pass > Testing WPF packing in ARGB32: pass > Testing WPF packing in XBGR32: pass > Testing WPF packing in XRGB32: pass > - vsp-unit-test-0002.sh > Testing WPF packing in NV12M: pass > Testing WPF packing in NV16M: pass > Testing WPF packing in NV21M: pass > Testing WPF packing in NV61M: pass > Testing WPF packing in UYVY: pass > Testing WPF packing in VYUY: skip > Testing WPF packing in YUV420M: pass > Testing WPF packing in YUV422M: pass > Testing WPF packing in YUV444M: pass > Testing WPF packing in YVU420M: pass > Testing WPF packing in YVU422M:
Re: [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb
Hi Kieran, Thank you for the patch. On Thursday, 8 March 2018 02:05:31 EEST Kieran Bingham wrote: > We are now able to configure a pipeline directly into a local display > list body. Take advantage of this fact, and create a cacheable body to > store the configuration of the pipeline in the video object. > > vsp1_video_pipeline_run() is now the last user of the pipe->dl object. > Convert this function to use the cached video->config body and obtain a > local display list reference. > > Attach the video->config body to the display list when needed before > committing to hardware. > > The pipe object is marked as un-configured when resuming from a suspend. > This ensures that when the hardware is reset - our cached configuration > will be re-attached to the next committed DL. > > Signed-off-by: Kieran Bingham> --- > > v3: > - 's/fragment/body/', 's/fragments/bodies/' > - video dlb cache allocation increased from 2 to 3 dlbs > > Our video DL usage now looks like the below output: > > dl->body0 contains our disposable runtime configuration. Max 41. > dl_child->body0 is our partition specific configuration. Max 12. > dl->bodies shows our constant configuration and LUTs. > > These two are LUT/CLU: > * dl->bodies[x]->num_entries 256 / max 256 > * dl->bodies[x]->num_entries 4914 / max 4914 > > Which shows that our 'constant' configuration cache is currently > utilised to a maximum of 64 entries. > > trace-cmd report | \ > grep max | sed 's/.*vsp1_dl_list_commit://g' | sort | uniq; > > dl->body0->num_entries 13 / max 128 > dl->body0->num_entries 14 / max 128 > dl->body0->num_entries 16 / max 128 > dl->body0->num_entries 20 / max 128 > dl->body0->num_entries 27 / max 128 > dl->body0->num_entries 34 / max 128 > dl->body0->num_entries 41 / max 128 > dl_child->body0->num_entries 10 / max 128 > dl_child->body0->num_entries 12 / max 128 > dl->bodies[x]->num_entries 15 / max 128 > dl->bodies[x]->num_entries 16 / max 128 > dl->bodies[x]->num_entries 17 / max 128 > dl->bodies[x]->num_entries 18 / max 128 > dl->bodies[x]->num_entries 20 / max 128 > dl->bodies[x]->num_entries 21 / max 128 > dl->bodies[x]->num_entries 256 / max 256 > dl->bodies[x]->num_entries 31 / max 128 > dl->bodies[x]->num_entries 32 / max 128 > dl->bodies[x]->num_entries 39 / max 128 > dl->bodies[x]->num_entries 40 / max 128 > dl->bodies[x]->num_entries 47 / max 128 > dl->bodies[x]->num_entries 48 / max 128 > dl->bodies[x]->num_entries 4914 / max 4914 > dl->bodies[x]->num_entries 55 / max 128 > dl->bodies[x]->num_entries 56 / max 128 > dl->bodies[x]->num_entries 63 / max 128 > dl->bodies[x]->num_entries 64 / max 128 This might be useful to capture in the main part of the commit message. > v4: > - Adjust pipe configured flag to be reset on resume rather than suspend > - rename dl_child, dl_next > > drivers/media/platform/vsp1/vsp1_pipe.c | 7 +++- > drivers/media/platform/vsp1/vsp1_pipe.h | 4 +- > drivers/media/platform/vsp1/vsp1_video.c | 67 - > drivers/media/platform/vsp1/vsp1_video.h | 2 +- > 4 files changed, 54 insertions(+), 26 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_pipe.c > b/drivers/media/platform/vsp1/vsp1_pipe.c index 5012643583b6..fa445b1a2e38 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_pipe.c > +++ b/drivers/media/platform/vsp1/vsp1_pipe.c > @@ -249,6 +249,7 @@ void vsp1_pipeline_run(struct vsp1_pipeline *pipe) > vsp1_write(vsp1, VI6_CMD(pipe->output->entity.index), > VI6_CMD_STRCMD); > pipe->state = VSP1_PIPELINE_RUNNING; > + pipe->configured = true; > } > > pipe->buffers_ready = 0; > @@ -470,6 +471,12 @@ void vsp1_pipelines_resume(struct vsp1_device *vsp1) > continue; > > spin_lock_irqsave(>irqlock, flags); > + /* > + * The hardware may have been reset during a suspend and will > + * need a full reconfiguration > + */ s/reconfiguration/reconfiguration./ > + pipe->configured = false; > + Where does that full reconfiguration occur, given that the vsp1_pipeline_run() right below sets pipe->configured to true without performing reconfiguration ? > if (vsp1_pipeline_ready(pipe)) > vsp1_pipeline_run(pipe); > spin_unlock_irqrestore(>irqlock, flags); > diff --git a/drivers/media/platform/vsp1/vsp1_pipe.h > b/drivers/media/platform/vsp1/vsp1_pipe.h index 90d29492b9b9..e7ad6211b4d0 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_pipe.h > +++ b/drivers/media/platform/vsp1/vsp1_pipe.h > @@ -90,6 +90,7 @@ struct vsp1_partition { > * @irqlock: protects the pipeline state > * @state: current state > * @wq: wait queue to wait for state change completion > + * @configured: flag determining if the hardware has run since reset >
Re: [PATCH v7 7/8] media: vsp1: Adapt entities to configure into a body
Hi Kieran, Thank you for the patch. On Thursday, 8 March 2018 02:05:30 EEST Kieran Bingham wrote: > Currently the entities store their configurations into a display list. > Adapt this such that the code can be configured into a body directly, > allowing greater flexibility and control of the content. > > All users of vsp1_dl_list_write() are removed in this process, thus it > too is removed. > > A helper, vsp1_dl_list_get_body0() is provided to access the internal body0 > from the display list. > > Signed-off-by: Kieran Bingham> > --- > v7 > - Rebase > - s/prepare/configure_stream/ > - s/configure/configure_frame/ > > drivers/media/platform/vsp1/vsp1_bru.c| 22 ++--- > drivers/media/platform/vsp1/vsp1_clu.c| 22 ++--- > drivers/media/platform/vsp1/vsp1_dl.c | 12 ++- > drivers/media/platform/vsp1/vsp1_dl.h | 2 +- > drivers/media/platform/vsp1/vsp1_drm.c| 20 +++ > drivers/media/platform/vsp1/vsp1_entity.c | 15 - > drivers/media/platform/vsp1/vsp1_entity.h | 11 +++--- > drivers/media/platform/vsp1/vsp1_hgo.c| 16 - > drivers/media/platform/vsp1/vsp1_hgt.c| 18 +- > drivers/media/platform/vsp1/vsp1_hsit.c | 10 +++--- > drivers/media/platform/vsp1/vsp1_lif.c| 15 - > drivers/media/platform/vsp1/vsp1_lut.c| 21 ++-- > drivers/media/platform/vsp1/vsp1_pipe.c | 4 +- > drivers/media/platform/vsp1/vsp1_pipe.h | 3 +- > drivers/media/platform/vsp1/vsp1_rpf.c| 39 +++--- > drivers/media/platform/vsp1/vsp1_sru.c| 14 > drivers/media/platform/vsp1/vsp1_uds.c| 24 +++--- > drivers/media/platform/vsp1/vsp1_uds.h| 2 +- > drivers/media/platform/vsp1/vsp1_video.c | 11 -- > drivers/media/platform/vsp1/vsp1_wpf.c| 42 > 20 files changed, 172 insertions(+), 151 deletions(-) [snip] > diff --git a/drivers/media/platform/vsp1/vsp1_uds.c > b/drivers/media/platform/vsp1/vsp1_uds.c index ce1731c2b3a9..6ddfce4bd095 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_uds.c > +++ b/drivers/media/platform/vsp1/vsp1_uds.c > @@ -31,22 +31,23 @@ > * Device Access > */ > > -static inline void vsp1_uds_write(struct vsp1_uds *uds, struct vsp1_dl_list > *dl, > - u32 reg, u32 data) > +static inline void vsp1_uds_write(struct vsp1_uds *uds, > + struct vsp1_dl_body *dlb, u32 reg, u32 data) > { > - vsp1_dl_list_write(dl, reg + uds->entity.index * VI6_UDS_OFFSET, data); > + vsp1_dl_body_write(dlb, reg + uds->entity.index * VI6_UDS_OFFSET, > +data); This can hold on a single line. > } [snip] > diff --git a/drivers/media/platform/vsp1/vsp1_video.c > b/drivers/media/platform/vsp1/vsp1_video.c index 1b5a31734834..b47708660e53 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_video.c > +++ b/drivers/media/platform/vsp1/vsp1_video.c [snip] > @@ -802,6 +804,9 @@ static int vsp1_video_setup_pipeline(struct > vsp1_pipeline *pipe) if (!pipe->dl) > return -ENOMEM; > > + /* Retrieve the default DLB from the list */ s/list/list./ > + dlb = vsp1_dl_list_get_body0(pipe->dl); > + > if (pipe->uds) { > struct vsp1_uds *uds = to_uds(>uds->subdev); > > @@ -824,8 +829,8 @@ static int vsp1_video_setup_pipeline(struct > vsp1_pipeline *pipe) } > > list_for_each_entry(entity, >entities, list_pipe) { > - vsp1_entity_route_setup(entity, pipe, pipe->dl); > - vsp1_entity_configure_stream(entity, pipe, pipe->dl); > + vsp1_entity_route_setup(entity, pipe, dlb); > + vsp1_entity_configure_stream(entity, pipe, dlb); > } > > return 0; > diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c > b/drivers/media/platform/vsp1/vsp1_wpf.c index 6a6cdf0fb5f1..68218625549e > 100644 > --- a/drivers/media/platform/vsp1/vsp1_wpf.c > +++ b/drivers/media/platform/vsp1/vsp1_wpf.c > @@ -31,9 +31,10 @@ > */ > > static inline void vsp1_wpf_write(struct vsp1_rwpf *wpf, > - struct vsp1_dl_list *dl, u32 reg, u32 data) > + struct vsp1_dl_body *dlb, u32 reg, u32 data) > { > - vsp1_dl_list_write(dl, reg + wpf->entity.index * VI6_WPF_OFFSET, data); > + vsp1_dl_body_write(dlb, reg + wpf->entity.index * VI6_WPF_OFFSET, > +data); This can hold on a single line. > } [snip] > @@ -292,10 +293,10 @@ static void wpf_configure_stream(struct vsp1_entity > *entity, > > wpf->outfmt = outfmt; > > - vsp1_dl_list_write(dl, VI6_DPR_WPF_FPORCH(wpf->entity.index), > -VI6_DPR_WPF_FPORCH_FP_WPFN); > + vsp1_dl_body_write(dlb, VI6_DPR_WPF_FPORCH(wpf->entity.index), > +VI6_DPR_WPF_FPORCH_FP_WPFN); Strange indentation. > > - vsp1_dl_list_write(dl, VI6_WPF_WRBCK_CTRL, 0); > +
Re: [PATCH v7 6/8] media: vsp1: Refactor display list configure operations
Hi Kieran, Thank you for the patch. On Thursday, 8 March 2018 02:05:29 EEST Kieran Bingham wrote: > The entities provide a single .configure operation which configures the > object into the target display list, based on the vsp1_entity_params > selection. > > This restricts us to a single function prototype for both static > configuration (the pre-stream INIT stage) and the dynamic runtime stages > for both each frame - and each partition therein. > > Split the configure function into two parts, '.configure_stream()' and > '.configure_frame()', merging both the VSP1_ENTITY_PARAMS_RUNTIME and > VSP1_ENTITY_PARAMS_PARTITION stages into a single call through the > .configure_frame(). The configuration for individual partitions is > handled by passing the partition number to the configure call, and > processing any runtime stage actions on the first partition only. > > Signed-off-by: Kieran Bingham> > --- > v7 > - Fix formatting and white space > - s/prepare/configure_stream/ > - s/configure/configure_frame/ > > drivers/media/platform/vsp1/vsp1_bru.c| 12 +- > drivers/media/platform/vsp1/vsp1_clu.c| 50 +--- > drivers/media/platform/vsp1/vsp1_dl.h | 1 +- > drivers/media/platform/vsp1/vsp1_drm.c| 21 +-- > drivers/media/platform/vsp1/vsp1_entity.c | 17 +- > drivers/media/platform/vsp1/vsp1_entity.h | 33 +-- > drivers/media/platform/vsp1/vsp1_hgo.c| 12 +- > drivers/media/platform/vsp1/vsp1_hgt.c| 12 +- > drivers/media/platform/vsp1/vsp1_hsit.c | 12 +- > drivers/media/platform/vsp1/vsp1_lif.c| 12 +- > drivers/media/platform/vsp1/vsp1_lut.c| 32 +- > drivers/media/platform/vsp1/vsp1_rpf.c| 164 ++--- > drivers/media/platform/vsp1/vsp1_sru.c| 12 +- > drivers/media/platform/vsp1/vsp1_uds.c| 57 ++-- > drivers/media/platform/vsp1/vsp1_video.c | 24 +-- > drivers/media/platform/vsp1/vsp1_wpf.c| 299 --- > 16 files changed, 378 insertions(+), 392 deletions(-) [snip] > diff --git a/drivers/media/platform/vsp1/vsp1_clu.c > b/drivers/media/platform/vsp1/vsp1_clu.c index b2a39a6ef7e4..b8d8af6d4910 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_clu.c > +++ b/drivers/media/platform/vsp1/vsp1_clu.c > @@ -213,37 +213,36 @@ static const struct v4l2_subdev_ops clu_ops = { > /* > --- > -- * VSP1 Entity Operations > */ > +static void clu_configure_stream(struct vsp1_entity *entity, > + struct vsp1_pipeline *pipe, > + struct vsp1_dl_list *dl) > +{ > + struct vsp1_clu *clu = to_clu(>subdev); > + > + /* > + * The yuv_mode can't be changed during streaming. Cache it internally > + * for future runtime configuration calls. > + */ I'd move this comment right before the vsp1_entity_get_pad_format() call to keep all variable declarations together. > + struct v4l2_mbus_framefmt *format; > + > + format = vsp1_entity_get_pad_format(>entity, > + clu->entity.config, > + CLU_PAD_SINK); > + clu->yuv_mode = format->code == MEDIA_BUS_FMT_AYUV8_1X32; > +} [snip] > diff --git a/drivers/media/platform/vsp1/vsp1_dl.h > b/drivers/media/platform/vsp1/vsp1_dl.h index 7e820ac6865a..f45083251644 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.h > +++ b/drivers/media/platform/vsp1/vsp1_dl.h > @@ -41,7 +41,6 @@ vsp1_dl_body_pool_create(struct vsp1_device *vsp1, > unsigned int num_bodies, void vsp1_dl_body_pool_destroy(struct > vsp1_dl_body_pool *pool); > struct vsp1_dl_body *vsp1_dl_body_get(struct vsp1_dl_body_pool *pool); > void vsp1_dl_body_put(struct vsp1_dl_body *dlb); > - This is an unrelated change. > void vsp1_dl_body_write(struct vsp1_dl_body *dlb, u32 reg, u32 data); > int vsp1_dl_list_add_body(struct vsp1_dl_list *dl, struct vsp1_dl_body > *dlb); > int vsp1_dl_list_add_chain(struct vsp1_dl_list *head, struct vsp1_dl_list > *dl); [snip] > diff --git a/drivers/media/platform/vsp1/vsp1_entity.h > b/drivers/media/platform/vsp1/vsp1_entity.h index > 408602ebeb97..b44ed5414fc3 100644 > --- a/drivers/media/platform/vsp1/vsp1_entity.h > +++ b/drivers/media/platform/vsp1/vsp1_entity.h [snip] > @@ -80,8 +68,10 @@ struct vsp1_route { > /** > * struct vsp1_entity_operations - Entity operations > * @destroy: Destroy the entity. > - * @configure: Setup the hardware based on the entity state (pipeline, > formats, > - * selection rectangles, ...) > + * @configure_stream:Setup the initial hardware parameters for the stream > + * (pipeline, formats) Instead of initial I would say "Setup hardware parameters that stay constant for the whole stream (pipeline, formats)", or possible "that don't vary between frames" instead. > + * @configure_frame: Configure the runtime parameters for each
Re: [PATCH v7 5/8] media: vsp1: Use reference counting for bodies
Hi Kieran, Thank you for the patch. On Thursday, 8 March 2018 02:05:28 EEST Kieran Bingham wrote: > Extend the display list body with a reference count, allowing bodies to > be kept as long as a reference is maintained. This provides the ability > to keep a cached copy of bodies which will not change, so that they can > be re-applied to multiple display lists. > > Signed-off-by: Kieran Bingham> > --- > This could be squashed into the body update code, but it's not a > straightforward squash as the refcounts will affect both: > v4l: vsp1: Provide a body pool > and > v4l: vsp1: Convert display lists to use new body pool > therefore, I have kept this separate to prevent breaking bisectability > of the vsp-tests. > > v3: > - 's/fragment/body/' > > v4: > - Fix up reference handling comments. > > drivers/media/platform/vsp1/vsp1_clu.c | 7 ++- > drivers/media/platform/vsp1/vsp1_dl.c | 15 ++- > drivers/media/platform/vsp1/vsp1_lut.c | 7 ++- > 3 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_clu.c > b/drivers/media/platform/vsp1/vsp1_clu.c index 2018144470c5..b2a39a6ef7e4 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_clu.c > +++ b/drivers/media/platform/vsp1/vsp1_clu.c > @@ -257,8 +257,13 @@ static void clu_configure(struct vsp1_entity *entity, > clu->clu = NULL; > spin_unlock_irqrestore(>lock, flags); > > - if (dlb) > + if (dlb) { > vsp1_dl_list_add_body(dl, dlb); > + > + /* release our local reference */ s/release/Release/ s/reference/reference./ > + vsp1_dl_body_put(dlb); > + } > + > break; > } > } > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c index 74476726451c..134865287c02 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -58,6 +59,8 @@ struct vsp1_dl_body { > struct list_head list; > struct list_head free; > > + refcount_t refcnt; > + > struct vsp1_dl_body_pool *pool; > struct vsp1_device *vsp1; > > @@ -259,6 +262,7 @@ struct vsp1_dl_body *vsp1_dl_body_get(struct > vsp1_dl_body_pool *pool) > if (!list_empty(>free)) { > dlb = list_first_entry(>free, struct vsp1_dl_body, free); > list_del(>free); > + refcount_set(>refcnt, 1); > } > > spin_unlock_irqrestore(>lock, flags); > @@ -279,6 +283,9 @@ void vsp1_dl_body_put(struct vsp1_dl_body *dlb) > if (!dlb) > return; > > + if (!refcount_dec_and_test(>refcnt)) > + return; > + > dlb->num_entries = 0; > > spin_lock_irqsave(>pool->lock, flags); > @@ -465,7 +472,11 @@ void vsp1_dl_list_write(struct vsp1_dl_list *dl, u32 > reg, u32 data) * in the order in which bodies are added. > * > * 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. > + * caller retains its reference to the fragment when adding it to the > display > + * list, but is not allowed to add new entries to the body. > + * > + * The reference must be explicitly released by a call to > vsp1_dl_body_put() > + * when the body isn't needed anymore. > * > * 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. @@ -476,6 +487,8 @@ int vsp1_dl_list_add_body(struct vsp1_dl_list > *dl, struct vsp1_dl_body *dlb) > if (dl->dlm->mode != VSP1_DL_MODE_HEADER) > return -EINVAL; > > + refcount_inc(>refcnt); > + > list_add_tail(>list, >bodies); > > return 0; > diff --git a/drivers/media/platform/vsp1/vsp1_lut.c > b/drivers/media/platform/vsp1/vsp1_lut.c index 262cb72139d6..77cf7137a0f2 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_lut.c > +++ b/drivers/media/platform/vsp1/vsp1_lut.c > @@ -213,8 +213,13 @@ static void lut_configure(struct vsp1_entity *entity, > lut->lut = NULL; > spin_unlock_irqrestore(>lock, flags); > > - if (dlb) > + if (dlb) { > vsp1_dl_list_add_body(dl, dlb); > + > + /* release our local reference */ s/release/Release/ s/reference/reference./ With these small issues fixed, Reviewed-by: Laurent Pinchart > + vsp1_dl_body_put(dlb); > + } > + > break; > } > } -- Regards, Laurent Pinchart
Re: [PATCH v7 4/8] media: vsp1: Convert display lists to use new body pool
Hi Kieran, Thank you for the patch. On Thursday, 8 March 2018 02:05:27 EEST Kieran Bingham wrote: > Adapt the dl->body0 object to use an object from the body 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. > > The CLU and LUT objects pre-allocate a pool containing three bodies, > allowing a userspace update before the hardware has committed a previous > set of tables. > > Bodies 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> > --- > v3: > - 's/fragment/body', 's/fragments/bodies/' > - CLU/LUT now allocate 3 bodies > - vsp1_dl_list_fragments_free -> vsp1_dl_list_bodies_put > > v2: > - Use dl->body0->max_entries to determine header offset, instead of the >global constant VSP1_DL_NUM_ENTRIES which is incorrect. > - squash updates for LUT, CLU, and fragment cleanup into single patch. >(Not fully bisectable when separated) > > drivers/media/platform/vsp1/vsp1_clu.c | 27 ++- > drivers/media/platform/vsp1/vsp1_clu.h | 1 +- > drivers/media/platform/vsp1/vsp1_dl.c | 223 ++ > drivers/media/platform/vsp1/vsp1_dl.h | 3 +- > drivers/media/platform/vsp1/vsp1_lut.c | 27 ++- > drivers/media/platform/vsp1/vsp1_lut.h | 1 +- > 6 files changed, 101 insertions(+), 181 deletions(-) Still a nice diffstart :-) [snip] > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c index 0208e72cb356..74476726451c > 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c [snip] > @@ -399,11 +311,10 @@ void vsp1_dl_body_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_body_pool *pool) Given that the only caller of this function passes dlm->pool as the second argument, can't you remove the second argument ? > { > struct vsp1_dl_list *dl; > - size_t header_size; > - int ret; > > dl = kzalloc(sizeof(*dl), GFP_KERNEL); > if (!dl) > @@ -412,41 +323,39 @@ static struct vsp1_dl_list *vsp1_dl_list_alloc(struct > vsp1_dl_manager *dlm) INIT_LIST_HEAD(>bodies); > 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 */ s/body pool/body pool./ (And I would have said "Get a body" but that's up to you) > + dl->body0 = vsp1_dl_body_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); > + size_t header_offset = dl->body0->max_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; > } > > +static void vsp1_dl_list_bodies_put(struct vsp1_dl_list *dl) > +{ > + struct vsp1_dl_body *dlb, *tmp; > + > + list_for_each_entry_safe(dlb, tmp, >bodies, list) { > + list_del(>list); > + vsp1_dl_body_put(dlb); > + } > +} > + > static void vsp1_dl_list_free(struct vsp1_dl_list *dl) > { > - vsp1_dl_body_cleanup(>body0); > - list_splice_init(>bodies, >dlm->gc_bodies); > + vsp1_dl_body_put(dl->body0); > + vsp1_dl_list_bodies_put(dl); Too bad we can't keep the list splice, it's more efficient than iterating over the list, but I suppose it's unavoidable if we want to reset the number of used entries to 0 for each body. Beside, we should have a small number of bodies only, so hopefully it
[PATCH v8 0/2] media: video-i2c: add video-i2c driver support
Add support for video-i2c polling driver Changes from v1: * Switch to SPDX tags versus GPLv2 license text * Remove unneeded zeroing of data structures * Add video_i2c_try_fmt_vid_cap call in video_i2c_s_fmt_vid_cap function Changes from v2: * Add missing linux/kthread.h include that broke x86_64 build Changes from v3: * Add devicetree binding documents * snprintf check added * switched to per chip support based on devicetree or i2c client id * add VB2_DMABUF to io_modes * added entry to MAINTAINERS file switched to per chip support based on devicetree or i2c client id Changes from v4: * convert pointer from of_device_get_match_data() to long instead of int to avoid compiler warning Changes from v5: * fix various issues with v4l2-compliance tool run Changes from v6: * fixed minor coding issues on spacing * changed device tree table pointers to chip struct data * add more verbose Kconfig documentation * destroy mutexes on error path and module removal * fixed MODULE_LICENSE from GPL to GPLv2 * changes some calls to list_last_entry() to avoid touching next pointer * moved common code to a function from start/stop_streaming() Changes from v7: * add const to several structs * corrected a few over 80 column lines * change DT check to generic dev_fwnode() call Matt Ranostay (2): media: dt-bindings: Add bindings for panasonic,amg88xx media: video-i2c: add video-i2c driver .../bindings/media/i2c/panasonic,amg88xx.txt | 19 + MAINTAINERS| 6 + drivers/media/i2c/Kconfig | 13 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/video-i2c.c | 563 + 5 files changed, 602 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt create mode 100644 drivers/media/i2c/video-i2c.c -- 2.14.1
[PATCH v8 1/2] media: dt-bindings: Add bindings for panasonic,amg88xx
Define the device tree bindings for the panasonic,amg88xx i2c video driver. Cc: devicet...@vger.kernel.org Reviewed-by: Rob HerringSigned-off-by: Matt Ranostay --- .../bindings/media/i2c/panasonic,amg88xx.txt | 19 +++ 1 file changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt diff --git a/Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt b/Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt new file mode 100644 index ..4a3181a3dd7e --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/panasonic,amg88xx.txt @@ -0,0 +1,19 @@ +* Panasonic AMG88xx + +The Panasonic family of AMG88xx Grid-Eye sensors allow recording +8x8 10Hz video which consists of thermal datapoints + +Required Properties: + - compatible : Must be "panasonic,amg88xx" + - reg : i2c address of the device + +Example: + + i2c0@1c22000 { + ... + amg88xx@69 { + compatible = "panasonic,amg88xx"; + reg = <0x69>; + }; + ... + }; -- 2.14.1
[PATCH v8 2/2] media: video-i2c: add video-i2c driver
There are several thermal sensors that only have a low-speed bus interface but output valid video data. This patchset enables support for the AMG88xx "Grid-Eye" sensor family. Signed-off-by: Matt Ranostay--- MAINTAINERS | 6 + drivers/media/i2c/Kconfig | 13 + drivers/media/i2c/Makefile| 1 + drivers/media/i2c/video-i2c.c | 563 ++ 4 files changed, 583 insertions(+) create mode 100644 drivers/media/i2c/video-i2c.c diff --git a/MAINTAINERS b/MAINTAINERS index dc153da22e8a..928b6a862626 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14880,6 +14880,12 @@ L: linux-media@vger.kernel.org S: Maintained F: drivers/media/platform/video-mux.c +VIDEO I2C POLLING DRIVER +M: Matt Ranostay +L: linux-media@vger.kernel.org +S: Maintained +F: drivers/media/i2c/video-i2c.c + VIDEOBUF2 FRAMEWORK M: Pawel Osciak M: Marek Szyprowski diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 541f0d28afd8..faaaceb94832 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -974,6 +974,19 @@ config VIDEO_M52790 To compile this driver as a module, choose M here: the module will be called m52790. + +config VIDEO_I2C + tristate "I2C transport video support" + depends on VIDEO_V4L2 && I2C + select VIDEOBUF2_VMALLOC + ---help--- + Enable the I2C transport video support which supports the + following: + * Panasonic AMG88xx Grid-Eye Sensors + + To compile this driver as a module, choose M here: the + module will be called video-i2c + endmenu menu "Sensors used on soc_camera driver" diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index ea34aee1a85a..84cc472238ef 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -96,6 +96,7 @@ obj-$(CONFIG_VIDEO_LM3646)+= lm3646.o obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o obj-$(CONFIG_VIDEO_AK881X) += ak881x.o obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o +obj-$(CONFIG_VIDEO_I2C)+= video-i2c.o obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o obj-$(CONFIG_VIDEO_OV2659) += ov2659.o obj-$(CONFIG_VIDEO_TC358743) += tc358743.o diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c new file mode 100644 index ..ec8a597597bf --- /dev/null +++ b/drivers/media/i2c/video-i2c.c @@ -0,0 +1,563 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * video-i2c.c - Support for I2C transport video devices + * + * Copyright (C) 2018 Matt Ranostay + * + * Supported: + * - Panasonic AMG88xx Grid-Eye Sensors + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define VIDEO_I2C_DRIVER "video-i2c" + +struct video_i2c_chip; + +struct video_i2c_buffer { + struct vb2_v4l2_buffer vb; + struct list_head list; +}; + +struct video_i2c_data { + struct i2c_client *client; + const struct video_i2c_chip *chip; + struct mutex lock; + spinlock_t slock; + unsigned int sequence; + struct mutex queue_lock; + + struct v4l2_device v4l2_dev; + struct video_device vdev; + struct vb2_queue vb_vidq; + + struct task_struct *kthread_vid_cap; + struct list_head vid_cap_active; +}; + +const static struct v4l2_fmtdesc amg88xx_format = { + .pixelformat = V4L2_PIX_FMT_Y12, +}; + +const static struct v4l2_frmsize_discrete amg88xx_size = { + .width = 8, + .height = 8, +}; + +struct video_i2c_chip { + /* video dimensions */ + const struct v4l2_fmtdesc *format; + const struct v4l2_frmsize_discrete *size; + + /* max frames per second */ + unsigned int max_fps; + + /* pixel buffer size */ + unsigned int buffer_size; + + /* pixel size in bits */ + unsigned int bpp; + + /* xfer function */ + int (*xfer)(struct video_i2c_data *data, char *buf); +}; + +static int amg88xx_xfer(struct video_i2c_data *data, char *buf) +{ + struct i2c_client *client = data->client; + struct i2c_msg msg[2]; + u8 reg = 0x80; + int ret; + + msg[0].addr = client->addr; + msg[0].flags = 0; + msg[0].len = 1; + msg[0].buf = (char *) + + msg[1].addr = client->addr; + msg[1].flags = I2C_M_RD; + msg[1].len = data->chip->buffer_size; + msg[1].buf = (char *)buf; + + ret = i2c_transfer(client->adapter, msg, 2); + + return (ret == 2) ? 0 : -EIO; +} + +#define AMG88XX0 + +static const struct video_i2c_chip video_i2c_chip[] = { + [AMG88XX] = { + .size = _size, +
Re: [PATCH v7 3/8] media: vsp1: Provide a body pool
Hi Kieran, Thank you for the patch. On Thursday, 8 March 2018 02:05:26 EEST Kieran Bingham wrote: > 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 I'd write it as "IOMMU TLB" to make it clear we're not concerned about CPU MMU TLB pressure. > 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 'body pool'. A pool can be allocated by the display list > manager or entities which require their own body allocations. > > Signed-off-by: Kieran Bingham> > --- > v4: > - Provide comment explaining extra allocation on body pool >highlighting area for optimisation later. > > v3: > - s/fragment/body/, s/fragments/bodies/ > - qty -> num_bodies > - indentation fix > - s/vsp1_dl_body_pool_{alloc,free}/vsp1_dl_body_pool_{create,destroy}/' > - Add kerneldoc to non-static functions > > v2: > - assign dlb->dma correctly > > drivers/media/platform/vsp1/vsp1_dl.c | 163 +++- > drivers/media/platform/vsp1/vsp1_dl.h | 8 +- > 2 files changed, 171 insertions(+) > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c index 67cc16c1b8e3..0208e72cb356 > 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -45,6 +45,8 @@ struct vsp1_dl_entry { > /** > * struct vsp1_dl_body - Display list body > * @list: entry in the display list list of bodies > + * @free: entry in the pool free body list Could we reuse @list for this purpose ? Unless I'm mistaken, when a body is in a pool it doesn't belong to any particular display list, and when it is in a display list it isn't in the pool anymore. > + * @pool: pool to which this body belongs > * @vsp1: the VSP1 device > * @entries: array of entries > * @dma: DMA address of the entries > @@ -54,6 +56,9 @@ struct vsp1_dl_entry { > */ > struct vsp1_dl_body { > struct list_head list; > + struct list_head free; > + > + struct vsp1_dl_body_pool *pool; > struct vsp1_device *vsp1; > > struct vsp1_dl_entry *entries; > @@ -65,6 +70,30 @@ struct vsp1_dl_body { > }; > > /** > + * struct vsp1_dl_body_pool - display list body 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 > + * @lock: Protects the pool and free list The pool and free list ? As far as I can tell the lock only protects the free list. > + * @vsp1: the VSP1 device > + */ > +struct vsp1_dl_body_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 > @@ -105,6 +134,7 @@ enum vsp1_dl_mode { > * @active: list currently being processed (loaded) by hardware > * @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 > * @gc_work: bodies garbage collector work struct > * @gc_bodies: array of display list bodies waiting to be freed > */ > @@ -120,6 +150,8 @@ struct vsp1_dl_manager { > struct vsp1_dl_list *queued; > struct vsp1_dl_list *pending; > > + struct vsp1_dl_body_pool *pool; > + > struct work_struct gc_work; > struct list_head gc_bodies; > }; > @@ -128,6 +160,137 @@ struct vsp1_dl_manager { > * Display List Body Management > */ > > +/** > + * vsp1_dl_body_pool_create - Create a pool of bodies from a single > allocation > + * @vsp1: The VSP1 device > + * @num_bodies: The quantity of bodies to allocate For consistency, s/quantity/number/ > + * @num_entries: The maximum number of entries that the body can contain Maybe s/the body/a body/ ? > + * @extra_size: Extra allocation provided for the bodies > + * > + * Allocate a pool of display list bodies each with enough memory to > contain the > + * requested number of entries. How about the requested number of entries plus the @extra_size. > + * > + * Return a pointer to a pool on success or NULL if memory can't be > allocated. > + */ > +struct vsp1_dl_body_pool * > +vsp1_dl_body_pool_create(struct vsp1_device *vsp1, unsigned int num_bodies, > + unsigned int num_entries, size_t extra_size) > +{ > + struct vsp1_dl_body_pool *pool; > +
Re: [PATCH v2 19/19] media: staging: davinci_vpfe: allow building with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on next-20180406] [cannot apply to v4.16] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1276:45: sparse: incorrect >> type in initializer (different address spaces) @@expected void [noderef] >> *from @@got void [noderef] *from @@ drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1276:45:expected void [noderef] *from drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1276:45:got void *[noderef] >> drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1319:43: sparse: incorrect >> type in initializer (different address spaces) @@expected void [noderef] >> *to @@got void [noderef] *to @@ drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1319:43:expected void [noderef] *to drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1319:43:got void *[noderef] >> drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1276:47: sparse: >> dereference of noderef expression drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1319:45: sparse: dereference of noderef expression -- >> drivers/staging/media/davinci_vpfe/dm365_resizer.c:922:32: sparse: incorrect >> type in argument 2 (different address spaces) @@expected void const >> [noderef] *from @@got stvoid const [noderef] *from @@ drivers/staging/media/davinci_vpfe/dm365_resizer.c:922:32:expected void const [noderef] *from drivers/staging/media/davinci_vpfe/dm365_resizer.c:922:32:got struct vpfe_rsz_config_params *config >> drivers/staging/media/davinci_vpfe/dm365_resizer.c:945:27: sparse: incorrect >> type in argument 1 (different address spaces) @@expected void [noderef] >> *to @@got sn:1>*to @@ drivers/staging/media/davinci_vpfe/dm365_resizer.c:945:27:expected void [noderef] *to drivers/staging/media/davinci_vpfe/dm365_resizer.c:945:27:got void * -- >> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: sparse: incorrect >> type in argument 2 (different address spaces) @@expected void volatile >> [noderef] *addr @@got sn:2>*addr @@ drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * >> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:201:27: sparse: incorrect >> type in assignment (different address spaces) @@expected void >> *ipipeif_base_addr @@got void [noderef] *ipipeif_base_addr >> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: sparse: incorrect >> type in argument 2 (different address spaces) @@expected void volatile >> [noderef] *addr @@got sn:2>*addr @@ drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * >> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: sparse: incorrect >> type in argument 2 (different address spaces) @@expected void volatile >> [noderef] *addr @@got sn:2>*addr @@ drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * >> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: sparse: incorrect >> type in argument 2 (different address spaces) @@expected void volatile >> [noderef] *addr @@got sn:2>*addr @@ drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * >> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: sparse: incorrect >> type in argument 2 (different address spaces) @@expected void volatile >> [noderef] *addr @@got sn:2>*addr @@ drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * >> drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: sparse: incorrect >> type in argument 2 (different address spaces) @@expected void volatile >> [noderef] *addr @@got sn:2>*addr @@ drivers/staging/media/davi
Re: Webcams not recognized on a Dell Latitude 5285 laptop
Hi Frédéric, On Thu, Mar 29, 2018 at 09:53:45AM +0200, FRÉDÉRIC PARRENIN wrote: > The second part now. I looked at the tables and it seems the dsdt lists two sensors (imx135 and ov2740) but the rest of the information on how they're connected etc. is missing. There are no sensor drivers in upstream kernel either. So there would be some work to do before you could capture raw bayer images from these devices. -- Kind regards, Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v7 1/8] media: vsp1: Reword uses of 'fragment' as 'body'
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> > --- > 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(>body0, reg, data); > + vsp1_dl_body_write(>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 -- Regards, Laurent Pinchart
Re: [RFCv9 PATCH 04/29] media-request: core request support
Hi Hans, Thanks for your work on this. A few comments below... On Wed, Mar 28, 2018 at 03:50:05PM +0200, Hans Verkuil wrote: > From: Hans Verkuil> > Implement the core of the media request processing. > > Drivers can bind request objects to a request. These objects > can then be marked completed if the driver finished using them, > or just be unbound if the results do not need to be kept (e.g. > in the case of buffers). > > Once all objects that were added are either unbound or completed, > the request is marked 'complete' and a POLLPRI signal is sent > via poll. > > Both requests and request objects are refcounted. > > While a request is queued its refcount is incremented (since it > is in use by a driver). Once it is completed the refcount is > decremented. When the user closes the request file descriptor > the refcount is also decremented. Once it reaches 0 all request > objects in the request are unbound and put() and the request > itself is freed. > > Signed-off-by: Hans Verkuil > --- > drivers/media/media-request.c | 269 > +- > include/media/media-request.h | 148 +++ > 2 files changed, 416 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c > index ead78613fdbe..8135d9d32af9 100644 > --- a/drivers/media/media-request.c > +++ b/drivers/media/media-request.c > @@ -16,8 +16,275 @@ > #include > #include > > +static const char * const request_state[] = { > + "idle", > + "queueing", > + "queued", > + "complete", > + "cleaning", > +}; > + > +static const char * > +media_request_state_str(enum media_request_state state) > +{ > + if (WARN_ON(state >= ARRAY_SIZE(request_state))) > + return "unknown"; > + return request_state[state]; > +} > + > +static void media_request_clean(struct media_request *req) > +{ > + struct media_request_object *obj, *obj_safe; > + > + WARN_ON(req->state != MEDIA_REQUEST_STATE_CLEANING); > + > + list_for_each_entry_safe(obj, obj_safe, >objects, list) { > + media_request_object_unbind(obj); > + media_request_object_put(obj); > + } > + > + req->num_incomplete_objects = 0; > + wake_up_interruptible(>poll_wait); > +} > + > +static void media_request_release(struct kref *kref) > +{ > + struct media_request *req = > + container_of(kref, struct media_request, kref); > + struct media_device *mdev = req->mdev; > + unsigned long flags; > + > + dev_dbg(mdev->dev, "request: release %s\n", req->debug_str); > + > + spin_lock_irqsave(>lock, flags); > + req->state = MEDIA_REQUEST_STATE_CLEANING; > + spin_unlock_irqrestore(>lock, flags); > + > + media_request_clean(req); > + > + if (mdev->ops->req_free) > + mdev->ops->req_free(req); > + else > + kfree(req); > +} > + > +void media_request_put(struct media_request *req) > +{ > + kref_put(>kref, media_request_release); > +} > +EXPORT_SYMBOL_GPL(media_request_put); > + > +static int media_request_close(struct inode *inode, struct file *filp) > +{ > + struct media_request *req = filp->private_data; > + > + media_request_put(req); One more newline? > + return 0; > +} > + > +static unsigned int media_request_poll(struct file *filp, > +struct poll_table_struct *wait) > +{ > + struct media_request *req = filp->private_data; > + unsigned long flags; > + enum media_request_state state; > + > + if (!(poll_requested_events(wait) & POLLPRI)) > + return 0; > + > + spin_lock_irqsave(>lock, flags); > + state = req->state; > + spin_unlock_irqrestore(>lock, flags); > + > + if (state == MEDIA_REQUEST_STATE_COMPLETE) > + return POLLPRI; > + if (state == MEDIA_REQUEST_STATE_IDLE) Should this be if (state != MEDIA_REQUEST_STATE_QUEUED) ? (Also see my other comment on the QUEUEING state below.) > + return POLLERR; > + > + poll_wait(filp, >poll_wait, wait); Newline? > + return 0; > +} > + > +static long media_request_ioctl(struct file *filp, unsigned int cmd, > + unsigned long __arg) > +{ > + return -ENOIOCTLCMD; > +} > + > +static const struct file_operations request_fops = { > + .owner = THIS_MODULE, > + .poll = media_request_poll, > + .unlocked_ioctl = media_request_ioctl, > + .release = media_request_close, > +}; > + > int media_request_alloc(struct media_device *mdev, > struct media_request_alloc *alloc) > { > - return -ENOMEM; > + struct media_request *req; > + struct file *filp; > + char comm[TASK_COMM_LEN]; > + int fd; > + int ret; > + > + fd = get_unused_fd_flags(O_CLOEXEC); > + if (fd < 0) > + return fd; > + > + filp = anon_inode_getfile("request", _fops,
[PATCH v2.1 09/15] v4l: vsp1: Move DRM pipeline output setup code to a function
In order to make the vsp1_du_setup_lif() easier to read, and for symmetry with the DRM pipeline input setup, move the pipeline output setup code to a separate function. Signed-off-by: Laurent PinchartReviewed-by: Kieran Bingham -- Changes since v2: - Moved vsp1_du_pipeline_setup_input() rename to earlier patch Changes since v1: - Rename vsp1_du_pipeline_setup_input() to vsp1_du_pipeline_setup_inputs() - Initialize format local variable to 0 in vsp1_du_pipeline_setup_output() --- drivers/media/platform/vsp1/vsp1_drm.c | 106 +++-- 1 file changed, 60 insertions(+), 46 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 4a628bbf7e47..a79b05ef 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -276,6 +276,65 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, return 0; } +/* Setup the output side of the pipeline (WPF and LIF). */ +static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1, +struct vsp1_pipeline *pipe) +{ + struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe); + struct v4l2_subdev_format format = { 0, }; + int ret; + + format.which = V4L2_SUBDEV_FORMAT_ACTIVE; + format.pad = RWPF_PAD_SINK; + format.format.width = drm_pipe->width; + format.format.height = drm_pipe->height; + format.format.code = MEDIA_BUS_FMT_ARGB_1X32; + format.format.field = V4L2_FIELD_NONE; + + ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n", + __func__, format.format.width, format.format.height, + format.format.code, pipe->output->entity.index); + + format.pad = RWPF_PAD_SOURCE; + ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n", + __func__, format.format.width, format.format.height, + format.format.code, pipe->output->entity.index); + + format.pad = LIF_PAD_SINK; + ret = v4l2_subdev_call(>lif->subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on LIF%u sink\n", + __func__, format.format.width, format.format.height, + format.format.code, pipe->lif->index); + + /* +* Verify that the format at the output of the pipeline matches the +* requested frame size and media bus code. +*/ + if (format.format.width != drm_pipe->width || + format.format.height != drm_pipe->height || + format.format.code != MEDIA_BUS_FMT_ARGB_1X32) { + dev_dbg(vsp1->dev, "%s: format mismatch on LIF%u\n", __func__, + pipe->lif->index); + return -EPIPE; + } + + return 0; +} + /* Configure all entities in the pipeline. */ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) { @@ -356,7 +415,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, struct vsp1_drm_pipeline *drm_pipe; struct vsp1_pipeline *pipe; struct vsp1_bru *bru; - struct v4l2_subdev_format format; unsigned long flags; unsigned int i; int ret; @@ -417,54 +475,10 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, if (ret < 0) return ret; - memset(, 0, sizeof(format)); - format.which = V4L2_SUBDEV_FORMAT_ACTIVE; - format.pad = RWPF_PAD_SINK; - format.format.width = cfg->width; - format.format.height = cfg->height; - format.format.code = MEDIA_BUS_FMT_ARGB_1X32; - format.format.field = V4L2_FIELD_NONE; - - ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL, - ); - if (ret < 0) - return ret; - - dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n", - __func__, format.format.width, format.format.height, - format.format.code, pipe->output->entity.index); - - format.pad = RWPF_PAD_SOURCE; - ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL, - ); - if (ret < 0) - return ret; - - dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n", - __func__, format.format.width, format.format.height, - format.format.code,
[PATCH v2.1 06/15] v4l: vsp1: Move DRM atomic commit pipeline setup to separate function
The DRM pipeline setup code used at atomic commit time is similar to the setup code used when enabling the pipeline. Move it to a separate function in order to share it. Signed-off-by: Laurent PinchartReviewed-by: Kieran Bingham --- Changes since v2: - Rename vsp1_du_pipeline_setup_input() to vsp1_du_pipeline_setup_inputs() --- drivers/media/platform/vsp1/vsp1_drm.c | 347 + 1 file changed, 180 insertions(+), 167 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 9a043a915c0b..d99278f45bd8 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -46,6 +46,185 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe, * Pipeline Configuration */ +/* Setup one RPF and the connected BRU sink pad. */ +static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1, + struct vsp1_pipeline *pipe, + struct vsp1_rwpf *rpf, + unsigned int bru_input) +{ + struct v4l2_subdev_selection sel; + struct v4l2_subdev_format format; + const struct v4l2_rect *crop; + int ret; + + /* +* Configure the format on the RPF sink pad and propagate it up to the +* BRU sink pad. +*/ + crop = >drm->inputs[rpf->entity.index].crop; + + memset(, 0, sizeof(format)); + format.which = V4L2_SUBDEV_FORMAT_ACTIVE; + format.pad = RWPF_PAD_SINK; + format.format.width = crop->width + crop->left; + format.format.height = crop->height + crop->top; + format.format.code = rpf->fmtinfo->mbus; + format.format.field = V4L2_FIELD_NONE; + + ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, + "%s: set format %ux%u (%x) on RPF%u sink\n", + __func__, format.format.width, format.format.height, + format.format.code, rpf->entity.index); + + memset(, 0, sizeof(sel)); + sel.which = V4L2_SUBDEV_FORMAT_ACTIVE; + sel.pad = RWPF_PAD_SINK; + sel.target = V4L2_SEL_TGT_CROP; + sel.r = *crop; + + ret = v4l2_subdev_call(>entity.subdev, pad, set_selection, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, + "%s: set selection (%u,%u)/%ux%u on RPF%u sink\n", + __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height, + rpf->entity.index); + + /* +* RPF source, hardcode the format to ARGB to turn on format +* conversion if needed. +*/ + format.pad = RWPF_PAD_SOURCE; + + ret = v4l2_subdev_call(>entity.subdev, pad, get_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, + "%s: got format %ux%u (%x) on RPF%u source\n", + __func__, format.format.width, format.format.height, + format.format.code, rpf->entity.index); + + format.format.code = MEDIA_BUS_FMT_ARGB_1X32; + + ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + /* BRU sink, propagate the format from the RPF source. */ + format.pad = bru_input; + + ret = v4l2_subdev_call(>bru->subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n", + __func__, format.format.width, format.format.height, + format.format.code, BRU_NAME(pipe->bru), format.pad); + + sel.pad = bru_input; + sel.target = V4L2_SEL_TGT_COMPOSE; + sel.r = vsp1->drm->inputs[rpf->entity.index].compose; + + ret = v4l2_subdev_call(>bru->subdev, pad, set_selection, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n", + __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height, + BRU_NAME(pipe->bru), sel.pad); + + return 0; +} + +static unsigned int rpf_zpos(struct vsp1_device *vsp1, struct vsp1_rwpf *rpf) +{ + return vsp1->drm->inputs[rpf->entity.index].zpos; +} + +/* Setup the input side of the pipeline (RPFs and BRU sink pads). */ +static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, +struct vsp1_pipeline *pipe) +{ + struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, }; + struct vsp1_bru *bru = to_bru(>bru->subdev); + unsigned int
Re: [PATCH v2 15/19] omap2: omapfb: allow building it with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 -- >> drivers/video/fbdev/omap2/omapfb/dss/dispc.c:289:9: sparse: context >> imbalance in 'mgr_fld_write' - different lock contexts for basic block vim +230 drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c f76ee892 Tomi Valkeinen 2015-12-09 222 f76ee892 Tomi Valkeinen 2015-12-09 223 static int panel_enabled(struct panel_drv_data *ddata) f76ee892 Tomi Valkeinen 2015-12-09 224 { f76ee892 Tomi Valkeinen 2015-12-09 225 u32 disp_status; f76ee892 Tomi Valkeinen 2015-12-09 226 int enabled; f76ee892 Tomi Valkeinen 2015-12-09 227 f76ee892 Tomi Valkeinen 2015-12-09 228 acx565akm_read(ddata, MIPID_CMD_READ_DISP_STATUS, f76ee892 Tomi Valkeinen 2015-12-09 229 (u8 *)_status, 4); f76ee892 Tomi Valkeinen 2015-12-09 @230 disp_status = __be32_to_cpu(disp_status); f76ee892 Tomi Valkeinen 2015-12-09 231 enabled = (disp_status & (1 << 17)) && (disp_status & (1 << 10)); f76ee892 Tomi Valkeinen 2015-12-09 232 dev_dbg(>spi->dev, f76ee892 Tomi Valkeinen 2015-12-09 233 "LCD panel %senabled by bootloader (status 0x%04x)\n", f76ee892 Tomi Valkeinen 2015-12-09 234 enabled ? "" : "not ", disp_status); f76ee892 Tomi Valkeinen 2015-12-09 235 return enabled; f76ee892 Tomi Valkeinen 2015-12-09 236 } f76ee892 Tomi Valkeinen 2015-12-09 237 :: The code at line 230 was first introduced by commit :: f76ee892a99e68b55402b8d4b8aeffcae2aff34d omapfb: copy omapdss & displays for omapfb :: TO: Tomi Valkeinen <tomi.valkei...@ti.com> :: CC: Tomi Valkeinen <tomi.valkei...@ti.com> --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-drivers-under-drivers-media-to-build-with-COMPILE_TEST/20180406-164215 base: git://linuxtv.org/media_tree.git master config: m68k-allyesconfig (attached as .config) compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k All warnings (new ones prefixed by >>): >> drivers/media//platform/fsl-viu.c:43:0: warning: "out_be32" redefined #define out_be32(v, a) writel(a, v) In file included from arch/m68k/include/asm/io_mm.h:27:0, from arch/m68k/include/asm/io.h:5, from include/linux/io.h:25, from drivers/media//platform/fsl-viu.c:23: arch/m68k/include/asm/raw_io.h:46:0: note: this is the location of the previous definition #define out_be32(addr,l) (void)((*(__force volatile u32 *) (addr)) = (l)) >> drivers/media//platform/fsl-viu.c:44:0: warning: "in_be32" redefined #define in_be32(a) readl(a) In file included from arch/m68k/include/asm/io_mm.h:27:0, from arch/m68k/include/asm/io.h:5, from include/linux/io.h:25, from drivers/media//platform/fsl-viu.c:23: arch/m68k/include/asm/raw_io.h:37:0: note: this is the location of the previous definition #define in_be32(addr) \ vim +/out_be32 +43 drivers/media//platform/fsl-viu.c > 23 #include 24 #include 25 #include 26 #include 27 #include 28 #include 29 #include 30 #include 31 #include 32 #include 33 #include 34 #include 35 36 #define DRV_NAME"fsl_viu" 37 #define VIU_VERSION "0.5.1" 38 39 /* Allow building this driver with COMPILE_TEST */ 40 #ifndef CONFIG_PPC_MPC512x 41 #define NO_IRQ 0 42 > 43 #define out_be32(v, a) writel(a, v) > 44 #define in_be32(a) readl(a) 45 #endif 46 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [RfC PATCH] Add udmabuf misc device
On Fri, Apr 06, 2018 at 03:36:03PM +0300, Oleksandr Andrushchenko wrote: > On 04/06/2018 02:57 PM, Gerd Hoffmann wrote: > > Hi, > > > >>>I fail to see any common ground for xen-zcopy and udmabuf ... > >>Does the above mean you can assume that xen-zcopy and udmabuf > >>can co-exist as two different solutions? > >Well, udmabuf route isn't fully clear yet, but yes. > > > >See also gvt (intel vgpu), where the hypervisor interface is abstracted > >away into a separate kernel modules even though most of the actual vgpu > >emulation code is common. > Thank you for your input, I'm just trying to figure out > which of the three z-copy solutions intersect and how much > >>And what about hyper-dmabuf? xen z-copy solution is pretty similar fundamentally to hyper_dmabuf in terms of these core sharing feature: 1. the sharing process - import prime/dmabuf from the producer -> extract underlying pages and get those shared -> return references for shared pages 2. the page sharing mechanism - it uses Xen-grant-table. And to give you a quick summary of differences as far as I understand between two implementations (please correct me if I am wrong, Oleksandr.) 1. xen-zcopy is DRM specific - can import only DRM prime buffer while hyper_dmabuf can export any dmabuf regardless of originator 2. xen-zcopy doesn't seem to have dma-buf synchronization between two VMs while (as danvet called it as remote dmabuf api sharing) hyper_dmabuf sends out synchronization message to the exporting VM for synchronization. 3. 1-level references - when using grant-table for sharing pages, there will be same # of refs (each 8 byte) as # of shared pages, which is passed to the userspace to be shared with importing VM in case of xen-zcopy. Compared to this, hyper_dmabuf does multiple level addressing to generate only one reference id that represents all shared pages. 4. inter VM messaging (hype_dmabuf only) - hyper_dmabuf has inter-vm msg communication defined for dmabuf synchronization and private data (meta info that Matt Roper mentioned) exchange. 5. driver-to-driver notification (hyper_dmabuf only) - importing VM gets notified when newdmabuf is exported from other VM - uevent can be optionally generated when this happens. 6. structure - hyper_dmabuf is targetting to provide a generic solution for inter-domain dmabuf sharing for most hypervisors, which is why it has two layers as mattrope mentioned, front-end that contains standard API and backend that is specific to hypervisor. > >No idea, didn't look at it in detail. > > > >Looks pretty complex from a distant view. Maybe because it tries to > >build a communication framework using dma-bufs instead of a simple > >dma-buf passing mechanism. we started with simple dma-buf sharing but realized there are many things we need to consider in real use-case, so we added communication , notification and dma-buf synchronization then re-structured it to front-end and back-end (this made things more compicated..) since Xen was not our only target. Also, we thought passing the reference for the buffer (hyper_dmabuf_id) is not secure so added uvent mechanism later. > Yes, I am looking at it now, trying to figure out the full story > and its implementation. BTW, Intel guys were about to share some > test application for hyper-dmabuf, maybe I have missed one. > It could probably better explain the use-cases and the complexity > they have in hyper-dmabuf. One example is actually in github. If you want take a look at it, please visit: https://github.com/downor/linux_hyper_dmabuf_test/tree/xen/simple_export > > > >Like xen-zcopy it seems to depend on the idea that the hypervisor > >manages all memory it is easy for guests to share pages with the help of > >the hypervisor. > So, for xen-zcopy we were not trying to make it generic, > it just solves display (dumb) zero-copying use-cases for Xen. > We implemented it as a DRM helper driver because we can't see any > other use-cases as of now. > For example, we also have Xen para-virtualized sound driver, but > its buffer memory usage is not comparable to what display wants > and it works somewhat differently (e.g. there is no "frame done" > event, so one can't tell when the sound buffer can be "flipped"). > At the same time, we do not use virtio-gpu, so this could probably > be one more candidate for shared dma-bufs some day. > > Which simply isn't the case on kvm. > > > >hyper-dmabuf and xen-zcopy could maybe share code, or hyper-dmabuf build > >on top of xen-zcopy. > Hm, I can imagine that: xen-zcopy could be a library code for hyper-dmabuf > in terms of implementing all that page sharing fun in multiple directions, > e.g. Host->Guest, Guest->Host, Guest<->Guest. > But I'll let Matt and Dongwon to comment on that. I think we can definitely collaborate. Especially, maybe we are using some outdated sharing mechanism/grant-table mechanism in our Xen backend (thanks for bringing that up Oleksandr). However, the question is once we collaborate
Re: [PATCH v2 12/19] media: davinci: allow build vpbe_display with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master config: x86_64-allmodconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All warnings (new ones prefixed by >>): In file included from drivers/media/platform/davinci/vpbe_display.c:35:0: drivers/media/platform/davinci/vpbe_display.c: In function 'register_device': >> drivers/media/platform/davinci/vpbe_display.c:1358:5: warning: cast from >> pointer to integer of different size [-Wpointer-to-int-cast] (int)vpbe_display_layer, ^ include/media/v4l2-common.h:69:44: note: in definition of macro 'v4l2_printk' printk(level "%s: " fmt, (dev)->name , ## arg) ^~~ >> drivers/media/platform/davinci/vpbe_display.c:1356:2: note: in expansion of >> macro 'v4l2_info' v4l2_info(_dev->vpbe_dev->v4l2_dev, ^ drivers/media/platform/davinci/vpbe_display.c:1359:5: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] (int)_display_layer->video_dev); ^ include/media/v4l2-common.h:69:44: note: in definition of macro 'v4l2_printk' printk(level "%s: " fmt, (dev)->name , ## arg) ^~~ >> drivers/media/platform/davinci/vpbe_display.c:1356:2: note: in expansion of >> macro 'v4l2_info' v4l2_info(_dev->vpbe_dev->v4l2_dev, ^ vim +1358 drivers/media/platform/davinci/vpbe_display.c a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1347 4c62e976 drivers/media/platform/davinci/vpbe_display.c Greg Kroah-Hartman 2012-12-21 1348 static int register_device(struct vpbe_layer *vpbe_display_layer, a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1349 struct vpbe_display *disp_dev, 4c62e976 drivers/media/platform/davinci/vpbe_display.c Greg Kroah-Hartman 2012-12-21 1350 struct platform_device *pdev) 4c62e976 drivers/media/platform/davinci/vpbe_display.c Greg Kroah-Hartman 2012-12-21 1351 { a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1352 int err; a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1353 a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1354 v4l2_info(_dev->vpbe_dev->v4l2_dev, a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1355"Trying to register VPBE display device.\n"); a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 @1356 v4l2_info(_dev->vpbe_dev->v4l2_dev, a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1357"layer=%x,layer->video_dev=%x\n", a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 @1358(int)vpbe_display_layer, a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1359(int)_display_layer->video_dev); a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1360 266c9c2d drivers/media/platform/davinci/vpbe_display.c Prabhakar Lad 2014-10-12 1361 vpbe_display_layer->video_dev.queue = _display_layer->buffer_queue; a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1362 err = video_register_device(_display_layer->video_dev, a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1363 VFL_TYPE_GRABBER, a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1364 -1); a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1365 if (err) a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1366 return -ENODEV; a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1367 a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1368 vpbe_display_layer->disp_dev = disp_dev; a2c25b44 drivers/media/video/davinci/vpbe_display.cManjunath Hadli 2011-06-17 1369 /*
uvcvideo stopped working in 4.16
Since the 4.16 kernel my uvcvideo webcam on Thinkpad X1 Carbon (5th gen) stopped working with gst-launch-1.0, kamoso (kde webcam app), Firefox and Chromium on sites like appear.in, talky.io, Google Hangouts and meet.jit.si. It works fine in 4.15 The camera is: Bus 001 Device 004: ID 04f2:b5ce Chicony Electronics Co., Ltd After further investigation, if I rmmod the uvcvideo module, and then load it again, the camera starts working normally. But I get this in dmesg: [ 63.399362] usbcore: deregistering interface driver uvcvideo [ 63.495659] WARNING: CPU: 1 PID: 858 at drivers/media/v4l2-core/v4l2-dev.c:176 v4l2_device_release+0xe3/0x100 [videodev] [ 63.495662] Modules linked in: rfcomm joydev mousedev rmi_smbus rmi_core bnep snd_hda_codec_hdmi snd_soc_skl snd_soc_skl_ipc snd_hda_codec_conexant snd_hda_ext_core snd_hda_codec_generic snd_soc_sst_dsp snd_soc_sst_ipc snd_soc_acpi snd_soc_core snd_compress ac97_bus snd_pcm_dmaengine arc4 iTCO_wdt iTCO_vendor_support wmi_bmof intel_wmi_thunderbolt intel_rapl iwlmvm x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel snd_hda_intel mac80211 snd_hda_codec kvm uvcvideo(-) btusb btrtl btbcm btintel snd_hda_core videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 iwlwifi videobuf2_common bluetooth snd_hwdep irqbypass e1000e intel_cstate snd_pcm videodev intel_uncore qcserial intel_rapl_perf cdc_mbim ptp usb_wwan psmouse pps_core snd_timer usbserial input_leds media pcspkr cdc_wdm cdc_ncm cfg80211 usbnet [ 63.495758] mii i2c_i801 rtsx_pci_ms memstick tpm_crb ecdh_generic mei_me mei shpchp intel_pch_thermal ucsi_acpi typec_ucsi thinkpad_acpi typec wmi nvram i2c_hid ac rfkill snd battery soundcore led_class tpm_tis rtc_cmos tpm_tis_core hid tpm evdev rng_core mac_hid usbip_host usbip_core sg scsi_mod crypto_user ip_tables x_tables ext4 crc16 mbcache jbd2 fscrypto algif_skcipher af_alg dm_crypt dm_mod crct10dif_pclmul crc32_pclmul crc32c_intel rtsx_pci_sdmmc ghash_clmulni_intel pcbc serio_raw mmc_core atkbd libps2 xhci_pci aesni_intel aes_x86_64 xhci_hcd crypto_simd glue_helper cryptd usbcore rtsx_pci usb_common i8042 serio vfat fat i915 intel_gtt i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm agpgart [ 63.495882] CPU: 1 PID: 858 Comm: rmmod Not tainted 4.16.0+ #1 [ 63.495885] Hardware name: LENOVO 20HQS0LV00/20HQS0LV00, BIOS N1MET45W (1.30 ) 02/22/2018 [ 63.495899] RIP: 0010:v4l2_device_release+0xe3/0x100 [videodev] [ 63.495903] RSP: 0018:9c974505bd60 EFLAGS: 00010202 [ 63.495907] RAX: RBX: 97d00b84a1d0 RCX: [ 63.495910] RDX: 97d00b84a018 RSI: 0286 RDI: c0c7f0e0 [ 63.495912] RBP: 97d008245ae0 R08: R09: [ 63.495915] R10: c823d22a9a00 R11: R12: 97d00b84a019 [ 63.495917] R13: 97d008245b90 R14: 97d00cbaf8f8 R15: [ 63.495921] FS: 7f3549e74b80() GS:97d01f48() knlGS: [ 63.495924] CS: 0010 DS: ES: CR0: 80050033 [ 63.495928] CR2: 01bffe78 CR3: 00047d4e2003 CR4: 003606e0 [ 63.495930] Call Trace: [ 63.495947] device_release+0x30/0x90 [ 63.495958] kobject_put+0x85/0x1a0 [ 63.495967] uvc_unregister_video+0x49/0x90 [uvcvideo] [ 63.495995] usb_unbind_interface+0x85/0x280 [usbcore] [ 63.496007] device_release_driver_internal+0x15a/0x220 [ 63.496014] driver_detach+0x37/0x70 [ 63.496021] bus_remove_driver+0x51/0xd0 [ 63.496051] usb_deregister+0x6a/0xd0 [usbcore] [ 63.496065] uvc_cleanup+0xc/0x36e [uvcvideo] [ 63.496075] SyS_delete_module+0x16a/0x2d0 [ 63.496088] do_syscall_64+0x74/0x190 [ 63.496100] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 [ 63.496105] RIP: 0033:0x7f35495975d7 [ 63.496108] RSP: 002b:7ffcbf653b88 EFLAGS: 0206 ORIG_RAX: 00b0 [ 63.496112] RAX: ffda RBX: RCX: 7f35495975d7 [ 63.496115] RDX: 000a RSI: 0800 RDI: 01bf57b8 [ 63.496117] RBP: 01bf5750 R08: 7ffcbf652b01 R09: [ 63.496119] R10: 08b2 R11: 0206 R12: 7ffcbf655d95 [ 63.496123] R13: R14: 01bf5750 R15: 01bf5260 [ 63.496127] Code: da ce 48 85 ed 74 1f 5b 48 89 ef 5d 41 5c e9 f5 6b 00 00 5b 5d 41 5c e9 6c 9c da ce 4c 89 e7 e8 04 80 f4 ff eb c6 5b 5d 41 5c c3 <0f> 0b 5b 5d 41 5c 48 c7 c7 e0 f0 c7 c0 e9 cb e8 ac ce 90 66 2e [ 63.496214] ---[ end trace d2f4fd290ae1befc ]--- [ 68.700338] uvcvideo: Found UVC 1.00 device Integrated Camera (04f2:b5ce) [ 68.707982] uvcvideo 1-8:1.0: Entity type for entity Extension 4 was not initialized! [ 68.707987] uvcvideo 1-8:1.0: Entity type for entity Extension 3 was not initialized! [ 68.707991] uvcvideo 1-8:1.0: Entity type for entity Processing 2 was not initialized! [ 68.707994] uvcvideo 1-8:1.0: Entity type for entity Camera 1 was not initialized! [
Re: [PATCH v2 18/19] media: si470x: allow build both USB and I2C at the same time
Em Sat, 7 Apr 2018 00:21:07 +0800 kbuild test robot <l...@intel.com> escreveu: > Hi Mauro, > > I love your patch! Yet something to improve: > > [auto build test ERROR on linuxtv-media/master] > [also build test ERROR on next-20180406] > [cannot apply to v4.16] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 > base: git://linuxtv.org/media_tree.git master > config: x86_64-federa-25 (attached as .config) > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All errors (new ones prefixed by >>): > >WARNING: modpost: missing MODULE_LICENSE() in > drivers/media/radio/si470x/radio-si470x-common.o >see include/linux/module.h for more information > >> ERROR: "si470x_set_freq" [drivers/media/radio/si470x/radio-si470x-usb.ko] > >> undefined! > >> ERROR: "si470x_viddev_template" > >> [drivers/media/radio/si470x/radio-si470x-usb.ko] undefined! > >> ERROR: "si470x_ctrl_ops" [drivers/media/radio/si470x/radio-si470x-usb.ko] > >> undefined! > >> ERROR: "si470x_stop" [drivers/media/radio/si470x/radio-si470x-usb.ko] > >> undefined! > >> ERROR: "si470x_start" [drivers/media/radio/si470x/radio-si470x-usb.ko] > >> undefined! > >> ERROR: "si470x_set_freq" [drivers/media/radio/si470x/radio-si470x-i2c.ko] > >> undefined! > >> ERROR: "si470x_viddev_template" > >> [drivers/media/radio/si470x/radio-si470x-i2c.ko] undefined! > >> ERROR: "si470x_ctrl_ops" [drivers/media/radio/si470x/radio-si470x-i2c.ko] > >> undefined! > >> ERROR: "si470x_start" [drivers/media/radio/si470x/radio-si470x-i2c.ko] > >> undefined! > >> ERROR: "si470x_stop" [drivers/media/radio/si470x/radio-si470x-i2c.ko] > >> undefined! > > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation Fixed patch enclosed. Thanks, Mauro [PATCH] media: si470x: allow build both USB and I2C at the same time Currently, either USB or I2C is built. Change it to allow having both enabled at the same time. The main reason is that COMPILE_TEST all[yes/mod]builds will now contain all drivers under drivers/media. Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com> diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig index 192f36f2f4aa..2ed539f9eb87 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -15,10 +15,6 @@ if RADIO_ADAPTERS && VIDEO_V4L2 config RADIO_TEA575X tristate -config RADIO_SI470X - bool "Silicon Labs Si470x FM Radio Receiver support" - depends on VIDEO_V4L2 - source "drivers/media/radio/si470x/Kconfig" config RADIO_SI4713 diff --git a/drivers/media/radio/si470x/Kconfig b/drivers/media/radio/si470x/Kconfig index a466654ee5c9..a21172e413a9 100644 --- a/drivers/media/radio/si470x/Kconfig +++ b/drivers/media/radio/si470x/Kconfig @@ -1,3 +1,17 @@ +config RADIO_SI470X +tristate "Silicon Labs Si470x FM Radio Receiver support" +depends on VIDEO_V4L2 + ---help--- + This is a driver for devices with the Silicon Labs SI470x + chip (either via USB or I2C buses). + + Say Y here if you want to connect this type of radio to your + computer's USB port or if it is used by some other driver + via I2C bus. + + To compile this driver as a module, choose M here: the + module will be called radio-si470x-common. + config USB_SI470X tristate "Silicon Labs Si470x FM Radio Receiver support with USB" depends on USB && RADIO_SI470X @@ -25,7 +39,7 @@ config USB_SI470X config I2C_SI470X tristate "Silicon Labs Si470x FM Radio Receiver support with I2C" - depends on I2C && RADIO_SI470X && !USB_SI470X + depends on I2C && RADIO_SI470X ---help--- This is a driver for I2C devices with the Silicon Labs SI470x chip. diff --git a/drivers/media/radio/si470x/Makefile b/drivers/media/radio/si470x/Makefile index 06964816cfd6..563500823e04 100644 --- a/drivers/media/radio/si470x/Makefile +++ b/drivers/media/radio/si470x/Makefile @@ -2,8 +2,6 @@ # Makefile for radios with Silicon Labs Si470x FM Radio Receivers # -radio-usb-si470x-objs := radio-si470x-usb.o radio-si470x-common.o -radio-i2c-si470
Re: [PATCH 17/21] media: ispstat: use %p to print the address of a buffer
Em Fri, 06 Apr 2018 18:46:05 +0300 Laurent Pinchartescreveu: > Hi Mauro, > > Thank you for the patch. > > On Friday, 6 April 2018 17:23:18 EEST Mauro Carvalho Chehab wrote: > > Instead of converting to int, use %p. That prevents this > > warning: > > drivers/media/platform/omap3isp/ispstat.c:451 isp_stat_bufs_alloc() > > warn: > > argument 7 to %08lx specifier is cast from pointer > > > > Signed-off-by: Mauro Carvalho Chehab > > --- > > drivers/media/platform/omap3isp/ispstat.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/platform/omap3isp/ispstat.c > > b/drivers/media/platform/omap3isp/ispstat.c index > > 47cbc7e3d825..eb1b589b0aeb 100644 > > --- a/drivers/media/platform/omap3isp/ispstat.c > > +++ b/drivers/media/platform/omap3isp/ispstat.c > > @@ -449,10 +449,10 @@ static int isp_stat_bufs_alloc(struct ispstat *stat, > > u32 size) buf->empty = 1; > > > > dev_dbg(stat->isp->dev, > > - "%s: buffer[%u] allocated. dma=0x%08lx virt=0x%08lx", > > + "%s: buffer[%u] allocated. dma=0x%08lx virt=%p", > > stat->subdev.name, i, > > (unsigned long)buf->dma_addr, > > - (unsigned long)buf->virt_addr); > > + buf->virt_addr); > > While at it you can use %pad for buf->dma_addr. > > dev_dbg(stat->isp->dev, > "%s: buffer[%u] allocated. dma=%pad virt=%p", > stat->subdev.name, i, >dma_addr, buf->virt_addr); > > With that change, > > Reviewed-by: Laurent Pinchart > > > } > > > > return 0; > OK. New patch enclosed. Thanks, Mauro [PATCH] media: ispstat: use %p to print the address of a buffer Instead of converting to int, use %p. That prevents this warning: drivers/media/platform/omap3isp/ispstat.c:451 isp_stat_bufs_alloc() warn: argument 7 to %08lx specifier is cast from pointer Reviewed-by: Laurent Pinchart Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c index 47cbc7e3d825..0b31f6c5791f 100644 --- a/drivers/media/platform/omap3isp/ispstat.c +++ b/drivers/media/platform/omap3isp/ispstat.c @@ -449,10 +449,8 @@ static int isp_stat_bufs_alloc(struct ispstat *stat, u32 size) buf->empty = 1; dev_dbg(stat->isp->dev, - "%s: buffer[%u] allocated. dma=0x%08lx virt=0x%08lx", - stat->subdev.name, i, - (unsigned long)buf->dma_addr, - (unsigned long)buf->virt_addr); + "%s: buffer[%u] allocated. dma=%pad virt=%p", + stat->subdev.name, i, >dma_addr, buf->virt_addr); } return 0;
Re: [PATCH 18/21] media: isppreview: fix __user annotations
Em Fri, 06 Apr 2018 18:54:50 +0300 Laurent Pinchartescreveu: > Hi Mauro, > > Thank you for the patch. > > On Friday, 6 April 2018 17:23:19 EEST Mauro Carvalho Chehab wrote: > > That prevent those warnings: > >drivers/media/platform/omap3isp/isppreview.c:893:45: warning: incorrect > > type in initializer (different address spaces) > > drivers/media/platform/omap3isp/isppreview.c:893:45:expected void > > [noderef] *from drivers/media/platform/omap3isp/isppreview.c:893:45: > >got void *[noderef] > > drivers/media/platform/omap3isp/isppreview.c:893:47: warning: dereference > > of noderef expression > > That's nice, but it would be even nicer to explain what the problem is and > how > you fix it, otherwise one might be left wondering if the fix is correct, or > if > it could be a false positive. Ok. Please see the enclosed patch. > With the commit message updated, > > Reviewed-by: Laurent Pinchart Thanks, Mauro [PATCH] media: isppreview: fix __user annotations The 'from' variable at preview_config() expects an __user * type. However, the logic there does: from = *(void * __user *) ((void *)cfg + attr->config_offset); With actually means a void pointer, pointing to a void __ user pointer. When the first pointer is de-referenced with *(foo), the type it returns is "void *" instead of "void __user *". Change it to: from = *(void __user **) ((void *)cfg + attr->config_offset); in order to obtain, when de-referenced, a void __user pointer, as desired. That prevent those warnings: drivers/media/platform/omap3isp/isppreview.c:893:45: warning: incorrect type in initializer (different address spaces) drivers/media/platform/omap3isp/isppreview.c:893:45:expected void [noderef] *from drivers/media/platform/omap3isp/isppreview.c:893:45:got void *[noderef] drivers/media/platform/omap3isp/isppreview.c:893:47: warning: dereference of noderef expression Reviewed-by: Laurent Pinchart Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/platform/omap3isp/isppreview.c b/drivers/media/platform/omap3isp/isppreview.c index ac30a0f83780..c2ef5870b231 100644 --- a/drivers/media/platform/omap3isp/isppreview.c +++ b/drivers/media/platform/omap3isp/isppreview.c @@ -890,7 +890,7 @@ static int preview_config(struct isp_prev_device *prev, params = >params.params[!!(active & bit)]; if (cfg->flag & bit) { - void __user *from = *(void * __user *) + void __user *from = *(void __user **) ((void *)cfg + attr->config_offset); void *to = (void *)params + attr->param_offset; size_t size = attr->param_size;
Re: [PATCH v2 18/19] media: si470x: allow build both USB and I2C at the same time
Hi Mauro, I love your patch! Yet something to improve: [auto build test ERROR on linuxtv-media/master] [also build test ERROR on next-20180406] [cannot apply to v4.16] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master config: x86_64-federa-25 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): WARNING: modpost: missing MODULE_LICENSE() in drivers/media/radio/si470x/radio-si470x-common.o see include/linux/module.h for more information >> ERROR: "si470x_set_freq" [drivers/media/radio/si470x/radio-si470x-usb.ko] >> undefined! >> ERROR: "si470x_viddev_template" >> [drivers/media/radio/si470x/radio-si470x-usb.ko] undefined! >> ERROR: "si470x_ctrl_ops" [drivers/media/radio/si470x/radio-si470x-usb.ko] >> undefined! >> ERROR: "si470x_stop" [drivers/media/radio/si470x/radio-si470x-usb.ko] >> undefined! >> ERROR: "si470x_start" [drivers/media/radio/si470x/radio-si470x-usb.ko] >> undefined! >> ERROR: "si470x_set_freq" [drivers/media/radio/si470x/radio-si470x-i2c.ko] >> undefined! >> ERROR: "si470x_viddev_template" >> [drivers/media/radio/si470x/radio-si470x-i2c.ko] undefined! >> ERROR: "si470x_ctrl_ops" [drivers/media/radio/si470x/radio-si470x-i2c.ko] >> undefined! >> ERROR: "si470x_start" [drivers/media/radio/si470x/radio-si470x-i2c.ko] >> undefined! >> ERROR: "si470x_stop" [drivers/media/radio/si470x/radio-si470x-i2c.ko] >> undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v2 11/19] media: davinci: allow building isif code
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/media/platform/davinci/isif.c:1066:22: sparse: incorrect type in >> assignment (different address spaces) @@expected void *[noderef] >> addr @@got void *[noderef] addr @@ drivers/media/platform/davinci/isif.c:1066:22:expected void *[noderef] addr drivers/media/platform/davinci/isif.c:1066:22:got void [noderef] * >> drivers/media/platform/davinci/isif.c:1074:44: sparse: incorrect type in >> assignment (different address spaces) @@expected void [noderef] >> *static [toplevel] [assigned] base_addr @@got [toplevel] >> [assigned] base_addr @@ drivers/media/platform/davinci/isif.c:1074:44:expected void [noderef] *static [toplevel] [assigned] base_addr drivers/media/platform/davinci/isif.c:1074:44:got void *[noderef] addr >> drivers/media/platform/davinci/isif.c:1078:51: sparse: incorrect type in >> assignment (different address spaces) @@expected void [noderef] >> *static [toplevel] [assigned] linear_tbl0_addr @@got [toplevel] >> [assigned] linear_tbl0_addr @@ drivers/media/platform/davinci/isif.c:1078:51:expected void [noderef] *static [toplevel] [assigned] linear_tbl0_addr drivers/media/platform/davinci/isif.c:1078:51:got void *[noderef] addr >> drivers/media/platform/davinci/isif.c:1082:51: sparse: incorrect type in >> assignment (different address spaces) @@expected void [noderef] >> *static [toplevel] [assigned] linear_tbl1_addr @@got [toplevel] >> [assigned] linear_tbl1_addr @@ drivers/media/platform/davinci/isif.c:1082:51:expected void [noderef] *static [toplevel] [assigned] linear_tbl1_addr drivers/media/platform/davinci/isif.c:1082:51:got void *[noderef] addr >> drivers/media/platform/davinci/isif.c:1067:22: sparse: dereference of >> noderef expression vim +1066 drivers/media/platform/davinci/isif.c 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1025 4c62e976 drivers/media/platform/davinci/isif.c Greg Kroah-Hartman 2012-12-21 1026 static int isif_probe(struct platform_device *pdev) 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1027 { 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1028 void (*setup_pinmux)(void); 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1029 struct resource *res; 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1030 void *__iomem addr; 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1031 int status = 0, i; 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1032 9a3e89b1 drivers/media/platform/davinci/isif.c Lad, Prabhakar 2013-03-22 1033 /* Platform data holds setup_pinmux function ptr */ 9a3e89b1 drivers/media/platform/davinci/isif.c Lad, Prabhakar 2013-03-22 1034 if (!pdev->dev.platform_data) 9a3e89b1 drivers/media/platform/davinci/isif.c Lad, Prabhakar 2013-03-22 1035 return -ENODEV; 9a3e89b1 drivers/media/platform/davinci/isif.c Lad, Prabhakar 2013-03-22 1036 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1037 /* 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1038 * first try to register with vpfe. If not correct platform, then we 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1039 * don't have to iomap 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1040 */ 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1041 status = vpfe_register_ccdc_device(_hw_dev); 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1042 if (status < 0) 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1043 return status; 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1044 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1045 setup_pinmux = pdev->dev.platform_data; 63e3ab14 drivers/media/video/davinci/isif.cMurali Karicheri 2010-02-21 1046 /
Re: [PATCH v2 11/15] v4l: vsp1: Add per-display list internal completion notification support
Hi Laurent, On 05/04/18 10:18, Laurent Pinchart wrote: > Display list completion is already reported to the frame end handler, > but that mechanism is global to all display lists. In order to implement > BRU and BRS reassignment in DRM pipelines we will need to commit a > display list and wait for its completion internally, without reporting > it to the DRM driver. Extend the display list API to support such an > internal use of the display list. > > Signed-off-by: Laurent Pinchart> --- > Changes since v1: > > - Use the frame end status flags to report notification > - Rename the notify flag to internal This reads much better to me. Thanks for the respin. Reviewed-by: Kieran Bingham > --- > drivers/media/platform/vsp1/vsp1_dl.c| 23 ++- > drivers/media/platform/vsp1/vsp1_dl.h| 3 ++- > drivers/media/platform/vsp1/vsp1_drm.c | 2 +- > drivers/media/platform/vsp1/vsp1_video.c | 2 +- > 4 files changed, 26 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c > index 662fa2a347c9..30ad491605ff 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -72,6 +72,7 @@ struct vsp1_dl_body { > * @fragments: list of extra display list bodies > * @has_chain: if true, indicates that there's a partition chain > * @chain: entry in the display list partition chain > + * @internal: whether the display list is used for internal purpose > */ > struct vsp1_dl_list { > struct list_head list; > @@ -85,6 +86,8 @@ struct vsp1_dl_list { > > bool has_chain; > struct list_head chain; > + > + bool internal; > }; > > enum vsp1_dl_mode { > @@ -550,8 +553,16 @@ static void vsp1_dl_list_commit_continuous(struct > vsp1_dl_list *dl) >* case we can't replace the queued list by the new one, as we could >* race with the hardware. We thus mark the update as pending, it will >* be queued up to the hardware by the frame end interrupt handler. > + * > + * If a display list is already pending we simply drop it as the new > + * display list is assumed to contain a more recent configuration. It is > + * an error if the already pending list has the internal flag set, as > + * there is then a process waiting for that list to complete. This > + * shouldn't happen as the waiting process should perform proper > + * locking, but warn just in case. >*/ > if (vsp1_dl_list_hw_update_pending(dlm)) { > + WARN_ON(dlm->pending && dlm->pending->internal); > __vsp1_dl_list_put(dlm->pending); > dlm->pending = dl; > return; > @@ -581,7 +592,7 @@ static void vsp1_dl_list_commit_singleshot(struct > vsp1_dl_list *dl) > dlm->active = dl; > } > > -void vsp1_dl_list_commit(struct vsp1_dl_list *dl) > +void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal) > { > struct vsp1_dl_manager *dlm = dl->dlm; > struct vsp1_dl_list *dl_child; > @@ -598,6 +609,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl) > } > } > > + dl->internal = internal; > + > spin_lock_irqsave(>lock, flags); > > if (dlm->singleshot) > @@ -624,6 +637,10 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl) > * raced with the frame end interrupt. The function always returns with the > flag > * set in header mode as display list processing is then not continuous and > * races never occur. > + * > + * The VSP1_DL_FRAME_END_INTERNAL flag indicates that the previous display > list > + * has completed and had been queued with the internal notification flag. > + * Internal notification is only supported for continuous mode. > */ > unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > { > @@ -656,6 +673,10 @@ unsigned int vsp1_dlm_irq_frame_end(struct > vsp1_dl_manager *dlm) >* frame end interrupt. The display list thus becomes active. >*/ > if (dlm->queued) { > + if (dlm->queued->internal) > + flags |= VSP1_DL_FRAME_END_INTERNAL; > + dlm->queued->internal = false; > + > __vsp1_dl_list_put(dlm->active); > dlm->active = dlm->queued; > dlm->queued = NULL; > diff --git a/drivers/media/platform/vsp1/vsp1_dl.h > b/drivers/media/platform/vsp1/vsp1_dl.h > index cbc2fc53e10b..1a5bbd5ddb7b 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.h > +++ b/drivers/media/platform/vsp1/vsp1_dl.h > @@ -21,6 +21,7 @@ struct vsp1_dl_list; > struct vsp1_dl_manager; > > #define VSP1_DL_FRAME_END_COMPLETED BIT(0) > +#define VSP1_DL_FRAME_END_INTERNAL BIT(1) > > void vsp1_dlm_setup(struct vsp1_device *vsp1); > > @@ -34,7 +35,7 @@ unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager >
Re: [PATCH v2 09/15] v4l: vsp1: Move DRM pipeline output setup code to a function
Hi Laurent, Thanks for the updates On 05/04/18 10:18, Laurent Pinchart wrote: > In order to make the vsp1_du_setup_lif() easier to read, and for > symmetry with the DRM pipeline input setup, move the pipeline output > setup code to a separate function. > > Signed-off-by: Laurent Pinchart> Reviewed-by: Kieran Bingham > -- > Changes since v1: > > - Rename vsp1_du_pipeline_setup_input() to > vsp1_du_pipeline_setup_inputs() Hrm ... I perhaps would have expected this to happen in [PATCH 06/15] v4l: vsp1: Move DRM atomic commit pipeline setup to separate function But I think that's being quite pedantic - so unless you have a need to respin, I wouldn't worry about it. -- Kieran > - Initialize format local variable to 0 in > vsp1_du_pipeline_setup_output() > --- > drivers/media/platform/vsp1/vsp1_drm.c | 114 > ++--- > 1 file changed, 64 insertions(+), 50 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > b/drivers/media/platform/vsp1/vsp1_drm.c > index 00ce99bd1605..a79b05ef 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -193,8 +193,8 @@ static unsigned int rpf_zpos(struct vsp1_device *vsp1, > struct vsp1_rwpf *rpf) > } > > /* Setup the input side of the pipeline (RPFs and BRU). */ > -static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1, > - struct vsp1_pipeline *pipe) > +static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, > + struct vsp1_pipeline *pipe) > { > struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, }; > struct vsp1_bru *bru = to_bru(>bru->subdev); > @@ -276,6 +276,65 @@ static int vsp1_du_pipeline_setup_input(struct > vsp1_device *vsp1, > return 0; > } > > +/* Setup the output side of the pipeline (WPF and LIF). */ > +static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1, > + struct vsp1_pipeline *pipe) > +{ > + struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe); > + struct v4l2_subdev_format format = { 0, }; > + int ret; > + > + format.which = V4L2_SUBDEV_FORMAT_ACTIVE; > + format.pad = RWPF_PAD_SINK; > + format.format.width = drm_pipe->width; > + format.format.height = drm_pipe->height; > + format.format.code = MEDIA_BUS_FMT_ARGB_1X32; > + format.format.field = V4L2_FIELD_NONE; > + > + ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL, > +); > + if (ret < 0) > + return ret; > + > + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n", > + __func__, format.format.width, format.format.height, > + format.format.code, pipe->output->entity.index); > + > + format.pad = RWPF_PAD_SOURCE; > + ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL, > +); > + if (ret < 0) > + return ret; > + > + dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n", > + __func__, format.format.width, format.format.height, > + format.format.code, pipe->output->entity.index); > + > + format.pad = LIF_PAD_SINK; > + ret = v4l2_subdev_call(>lif->subdev, pad, set_fmt, NULL, > +); > + if (ret < 0) > + return ret; > + > + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on LIF%u sink\n", > + __func__, format.format.width, format.format.height, > + format.format.code, pipe->lif->index); > + > + /* > + * Verify that the format at the output of the pipeline matches the > + * requested frame size and media bus code. > + */ > + if (format.format.width != drm_pipe->width || > + format.format.height != drm_pipe->height || > + format.format.code != MEDIA_BUS_FMT_ARGB_1X32) { > + dev_dbg(vsp1->dev, "%s: format mismatch on LIF%u\n", __func__, > + pipe->lif->index); > + return -EPIPE; > + } > + > + return 0; > +} > + > /* Configure all entities in the pipeline. */ > static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) > { > @@ -356,7 +415,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int > pipe_index, > struct vsp1_drm_pipeline *drm_pipe; > struct vsp1_pipeline *pipe; > struct vsp1_bru *bru; > - struct v4l2_subdev_format format; > unsigned long flags; > unsigned int i; > int ret; > @@ -413,58 +471,14 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int > pipe_index, > __func__, pipe_index, cfg->width, cfg->height); > > /* Setup formats through the pipeline. */ > - ret = vsp1_du_pipeline_setup_input(vsp1, pipe); > - if (ret < 0) > -
Re: [PATCH v2 10/15] v4l: vsp1: Turn frame end completion status into a bitfield
Hi Laurent, Thanks for this enhancement. On 05/04/18 10:18, Laurent Pinchart wrote: > We will soon need to return more than a boolean completion status from > the vsp1_dlm_irq_frame_end() IRQ handler. Turn the return value into a > bitfield to prepare for that. No functional change is introduced here. I think this is a good solution! Reviewed-by: Kieran Bingham> Signed-off-by: Laurent Pinchart > --- > drivers/media/platform/vsp1/vsp1_dl.c| 22 +- > drivers/media/platform/vsp1/vsp1_dl.h| 4 +++- > drivers/media/platform/vsp1/vsp1_drm.c | 5 +++-- > drivers/media/platform/vsp1/vsp1_pipe.c | 8 > drivers/media/platform/vsp1/vsp1_pipe.h | 2 +- > drivers/media/platform/vsp1/vsp1_video.c | 4 ++-- > 6 files changed, 26 insertions(+), 19 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c > index 0b86ed01e85d..662fa2a347c9 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -616,14 +616,18 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl) > * vsp1_dlm_irq_frame_end - Display list handler for the frame end interrupt > * @dlm: the display list manager > * > - * Return true if the previous display list has completed at frame end, or > false > - * if it has been delayed by one frame because the display list commit raced > - * with the frame end interrupt. The function always returns true in header > mode > - * as display list processing is then not continuous and races never occur. > + * Return a set of flags that indicates display list completion status. > + * > + * The VSP1_DL_FRAME_END_COMPLETED flag indicates that the previous display > list > + * has completed at frame end. If the flag is not returned display list > + * completion has been delayed by one frame because the display list commit > + * raced with the frame end interrupt. The function always returns with the > flag > + * set in header mode as display list processing is then not continuous and > + * races never occur. > */ > -bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > +unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > { > - bool completed = false; > + unsigned int flags = 0; > > spin_lock(>lock); > > @@ -634,7 +638,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > if (dlm->singleshot) { > __vsp1_dl_list_put(dlm->active); > dlm->active = NULL; > - completed = true; > + flags |= VSP1_DL_FRAME_END_COMPLETED; > goto done; > } > > @@ -655,7 +659,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > __vsp1_dl_list_put(dlm->active); > dlm->active = dlm->queued; > dlm->queued = NULL; > - completed = true; > + flags |= VSP1_DL_FRAME_END_COMPLETED; > } > > /* > @@ -672,7 +676,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > done: > spin_unlock(>lock); > > - return completed; > + return flags; > } > > /* Hardware Setup */ > diff --git a/drivers/media/platform/vsp1/vsp1_dl.h > b/drivers/media/platform/vsp1/vsp1_dl.h > index ee3508172f0a..cbc2fc53e10b 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.h > +++ b/drivers/media/platform/vsp1/vsp1_dl.h > @@ -20,6 +20,8 @@ struct vsp1_dl_fragment; > struct vsp1_dl_list; > struct vsp1_dl_manager; > > +#define VSP1_DL_FRAME_END_COMPLETED BIT(0) > + > void vsp1_dlm_setup(struct vsp1_device *vsp1); > > struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1, > @@ -27,7 +29,7 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device > *vsp1, > unsigned int prealloc); > void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm); > void vsp1_dlm_reset(struct vsp1_dl_manager *dlm); > -bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm); > +unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm); > > struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm); > void vsp1_dl_list_put(struct vsp1_dl_list *dl); > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > b/drivers/media/platform/vsp1/vsp1_drm.c > index a79b05ef..541473b1df67 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -34,12 +34,13 @@ > */ > > static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe, > -bool completed) > +unsigned int completion) > { > struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe); > > if (drm_pipe->du_complete) > - drm_pipe->du_complete(drm_pipe->du_private, completed); > + drm_pipe->du_complete(drm_pipe->du_private, > +
Re: [PATCH] media: coda: do not try to propagate format if capture queue busy
Hi Tomasz, On Tue, 2018-04-03 at 10:13 +, Tomasz Figa wrote: > Hi Philipp, > > On Thu, Mar 29, 2018 at 2:12 AM Philipp Zabel> wrote: > > > The driver helpfully resets the capture queue format and selection > > rectangle whenever output format is changed. This only works while > > the capture queue is not busy. > > Is the code in question used only for decoder case? For encoder, CAPTURE > queue determines the codec and so things should work the other way around, > i.e. setting CAPTURE format should reset OUTPUT format and it should be > allowed only if OUTPUT queue is not busy. thank you for the comment, this code is indeed also used for the encoder device. I'll fix this in the next round. regards Philipp
Re: [PATCH] media: coda: do not try to propagate format if capture queue busy
Hi Ian, On Fri, 2018-04-06 at 09:40 +0100, Ian Arkver wrote: > > - ret = coda_try_fmt_vid_cap(file, priv, _cap); > > - if (ret) > > - return ret; > > - > > - q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT); > > - r.left = 0; > > - r.top = 0; > > - r.width = q_data_src->width; > > - r.height = q_data_src->height; > > - > > - return coda_s_fmt(ctx, _cap, ); > > + return coda_s_fmt_vid_cap(file, priv, _cap); > > Is this chunk (and removal of q_data_src above) part of the same patch? > It doesn't seem covered by the subject line. You are right, I'll move this into a separate patch. thanks Philipp
Re: [PATCH 18/21] media: isppreview: fix __user annotations
Hi Mauro, Thank you for the patch. On Friday, 6 April 2018 17:23:19 EEST Mauro Carvalho Chehab wrote: > That prevent those warnings: >drivers/media/platform/omap3isp/isppreview.c:893:45: warning: incorrect > type in initializer (different address spaces) > drivers/media/platform/omap3isp/isppreview.c:893:45:expected void > [noderef] *from drivers/media/platform/omap3isp/isppreview.c:893:45: >got void *[noderef] > drivers/media/platform/omap3isp/isppreview.c:893:47: warning: dereference > of noderef expression That's nice, but it would be even nicer to explain what the problem is and how you fix it, otherwise one might be left wondering if the fix is correct, or if it could be a false positive. With the commit message updated, Reviewed-by: Laurent Pinchart> Signed-off-by: Mauro Carvalho Chehab > --- > drivers/media/platform/omap3isp/isppreview.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/omap3isp/isppreview.c > b/drivers/media/platform/omap3isp/isppreview.c index > ac30a0f83780..c2ef5870b231 100644 > --- a/drivers/media/platform/omap3isp/isppreview.c > +++ b/drivers/media/platform/omap3isp/isppreview.c > @@ -890,7 +890,7 @@ static int preview_config(struct isp_prev_device *prev, > params = >params.params[!!(active & bit)]; > > if (cfg->flag & bit) { > - void __user *from = *(void * __user *) > + void __user *from = *(void __user **) > ((void *)cfg + attr->config_offset); > void *to = (void *)params + attr->param_offset; > size_t size = attr->param_size; -- Regards, Laurent Pinchart
Re: [PATCH 17/21] media: ispstat: use %p to print the address of a buffer
Hi Mauro, Thank you for the patch. On Friday, 6 April 2018 17:23:18 EEST Mauro Carvalho Chehab wrote: > Instead of converting to int, use %p. That prevents this > warning: > drivers/media/platform/omap3isp/ispstat.c:451 isp_stat_bufs_alloc() > warn: > argument 7 to %08lx specifier is cast from pointer > > Signed-off-by: Mauro Carvalho Chehab> --- > drivers/media/platform/omap3isp/ispstat.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/omap3isp/ispstat.c > b/drivers/media/platform/omap3isp/ispstat.c index > 47cbc7e3d825..eb1b589b0aeb 100644 > --- a/drivers/media/platform/omap3isp/ispstat.c > +++ b/drivers/media/platform/omap3isp/ispstat.c > @@ -449,10 +449,10 @@ static int isp_stat_bufs_alloc(struct ispstat *stat, > u32 size) buf->empty = 1; > > dev_dbg(stat->isp->dev, > - "%s: buffer[%u] allocated. dma=0x%08lx virt=0x%08lx", > + "%s: buffer[%u] allocated. dma=0x%08lx virt=%p", > stat->subdev.name, i, > (unsigned long)buf->dma_addr, > - (unsigned long)buf->virt_addr); > + buf->virt_addr); While at it you can use %pad for buf->dma_addr. dev_dbg(stat->isp->dev, "%s: buffer[%u] allocated. dma=%pad virt=%p", stat->subdev.name, i, >dma_addr, buf->virt_addr); With that change, Reviewed-by: Laurent Pinchart > } > > return 0; -- Regards, Laurent Pinchart
[PATCH 2/2] media: omapfb: relax compilation if COMPILE_TEST
The dependency of DRM_OMAP = n can be relaxed for just compilation test. This allows building the omap3isp driver with allyesconfig on ARM. Signed-off-by: Mauro Carvalho Chehab--- drivers/video/fbdev/omap2/omapfb/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/omap2/omapfb/Kconfig b/drivers/video/fbdev/omap2/omapfb/Kconfig index e6226aeed17e..e42794a5e26c 100644 --- a/drivers/video/fbdev/omap2/omapfb/Kconfig +++ b/drivers/video/fbdev/omap2/omapfb/Kconfig @@ -4,7 +4,7 @@ config OMAP2_VRFB menuconfig FB_OMAP2 tristate "OMAP2+ frame buffer support" depends on FB -depends on DRM_OMAP = n +depends on DRM_OMAP = n || COMPILE_TEST select FB_OMAP2_DSS select OMAP2_VRFB if ARCH_OMAP2 || ARCH_OMAP3 -- 2.14.3
[PATCH 1/2] media: meye: relax dependencies if COMPILE_TEST
This driver can be built successfuly on non-x86 archs, if we remove SONY_LAPTOP dependency. Signed-off-by: Mauro Carvalho Chehab--- drivers/media/pci/meye/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/meye/Kconfig b/drivers/media/pci/meye/Kconfig index b4bf848be5a0..2e60334ffef5 100644 --- a/drivers/media/pci/meye/Kconfig +++ b/drivers/media/pci/meye/Kconfig @@ -1,6 +1,7 @@ config VIDEO_MEYE tristate "Sony Vaio Picturebook Motion Eye Video For Linux" - depends on PCI && SONY_LAPTOP && VIDEO_V4L2 + depends on PCI && VIDEO_V4L2 + depends on SONY_LAPTOP || COMPILE_TEST ---help--- This is the video4linux driver for the Motion Eye camera found in the Vaio Picturebook laptops. Please read the material in -- 2.14.3
Re: [PATCH v2 09/19] media: marvel-ccic: re-enable mmp-driver build
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/media/platform/marvell-ccic/mmp-driver.c:135:41: sparse: incorrect >> type in argument 2 (different address spaces) @@expected void [noderef] >> * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:135:41:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:135:41:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:136:44: sparse: incorrect type in argument 2 (different address spaces) @@expected void [noderef] * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:136:44:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:136:44:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:174:38: sparse: incorrect type in argument 2 (different address spaces) @@expected void [noderef] * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:174:38:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:174:38:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:175:38: sparse: incorrect type in argument 2 (different address spaces) @@expected void [noderef] * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:175:38:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:175:38:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:195:48: sparse: incorrect type in argument 1 (different address spaces) @@expected void [noderef] * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:195:48:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:195:48:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:196:55: sparse: incorrect type in argument 2 (different address spaces) @@expected void [noderef] * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:196:55:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:196:55:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:197:54: sparse: incorrect type in argument 2 (different address spaces) @@expected void [noderef] * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:197:54:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:197:54:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:202:48: sparse: incorrect type in argument 1 (different address spaces) @@expected void [noderef] * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:202:48:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:202:48:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:203:55: sparse: incorrect type in argument 2 (different address spaces) @@expected void [noderef] * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:203:55:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:203:55:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:204:54: sparse: incorrect type in argument 2 (different address spaces) @@expected void [noderef] * @@got sn:2>* @@ drivers/media/platform/marvell-ccic/mmp-driver.c:204:54:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:204:54:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:186:6: sparse: symbol 'mcam_ctlr_reset' was not declared. Should it be static? drivers/media/platform/marvell-ccic/mmp-driver.c:217:6: sparse: symbol 'mmpcam_calc_dphy' was not declared. Should it be static? >> drivers/media/platform/marvell-ccic/mmp-driver.c:389:25: sparse: incorrect >> type in assignment (different address spaces) @@expected void >> *power_regs @@got void [noderef] * vim +135 drivers/media/platform/marvell-ccic/mmp-driver.c 0e394f44f drivers/media/platform/marvell-ccic/mmp-driver.c Libin Yang 2013-07-03 129 67a8dbbc4 drivers/media/video/marvell-ccic/mmp-driver.cJonathan Corbet 2011-06-11 130 /* 67a8dbbc4 drivers/media/video/marvell-ccic/mmp-driver.cJonathan Corbet 2011-06-11 131 * Power control. 67a8dbbc4 drivers/media/video/marvell-ccic/mmp-driver.cJonathan Cor
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Em Fri, 6 Apr 2018 16:37:15 +0200 Arnd Bergmannescreveu: > On Fri, Apr 6, 2018 at 4:26 PM, Mauro Carvalho Chehab > wrote: > > Em Fri, 6 Apr 2018 16:16:46 +0200 > > Arnd Bergmann escreveu: > > > >> On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehab > >> wrote: > >> > Em Fri, 6 Apr 2018 11:51:16 +0200 > >> > Arnd Bergmann escreveu: > >> > > >> >> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab > >> >> wrote: > >> >> > >> >> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST > >> >> > > >> >> > There aren't many things that would be needed to allow it > >> >> > to build with compile test. > >> >> > > >> >> > Add the needed bits. > >> >> > > >> >> > Signed-off-by: Mauro Carvalho Chehab > >> >> > >> >> Reviewed-by: Arnd Bergmann > >> > > >> > Actually, in order to avoid warnings with smatch, the COMPILE_TEST > >> > macros should be declared as: > >> > > >> > +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) > >> > +#define in_be32(a) ioread32be((void __iomem *)a) > >> > >> I would just add the correct annotations, I think they've always been > >> missing. > >> 2 patches coming in a few minutes. > > > > I corrected the annotations too. Now, it gives the same results > > building for both arm and x86. > > > > If you want to double check, the full tree is at: > > > > > > https://git.linuxtv.org/mchehab/experimental.git/log/?h=compile_test > > The __iomem annotations look good, my other patch is still needed to > get a clean build with "make C=1" but doesn't apply cleanly on top of your > version. I assume you'll just fix it up accordingly. Heh, another duplicated patch: https://git.linuxtv.org/mchehab/experimental.git/commit/?h=compile_test=687520dc31a88c82c694492423c5d9c503cbdebb That's why it didn't apply cleanly: $ patch -p1 -i /tmp/media\:\ platform\:\ fsl-viu\:\ mark\ local\ functions\ \'static\'.patch --merge patching file drivers/media/platform/fsl-viu.c Hunk #1 already applied at 238. Hunk #2 already applied at 251. Hunk #3 already applied at 262. Hunk #4 already applied at 806. Hunk #5 already applied at 817. Hunk #6 already applied at 1305. Great minds think alike :-) Thanks, Mauro
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
On Fri, Apr 6, 2018 at 4:26 PM, Mauro Carvalho Chehabwrote: > Em Fri, 6 Apr 2018 16:16:46 +0200 > Arnd Bergmann escreveu: > >> On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehab >> wrote: >> > Em Fri, 6 Apr 2018 11:51:16 +0200 >> > Arnd Bergmann escreveu: >> > >> >> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab >> >> wrote: >> >> >> >> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST >> >> > >> >> > There aren't many things that would be needed to allow it >> >> > to build with compile test. >> >> > >> >> > Add the needed bits. >> >> > >> >> > Signed-off-by: Mauro Carvalho Chehab >> >> >> >> Reviewed-by: Arnd Bergmann >> > >> > Actually, in order to avoid warnings with smatch, the COMPILE_TEST >> > macros should be declared as: >> > >> > +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) >> > +#define in_be32(a) ioread32be((void __iomem *)a) >> >> I would just add the correct annotations, I think they've always been >> missing. >> 2 patches coming in a few minutes. > > I corrected the annotations too. Now, it gives the same results > building for both arm and x86. > > If you want to double check, the full tree is at: > > https://git.linuxtv.org/mchehab/experimental.git/log/?h=compile_test The __iomem annotations look good, my other patch is still needed to get a clean build with "make C=1" but doesn't apply cleanly on top of your version. I assume you'll just fix it up accordingly. Arnd
Re: [PATCH 1/2] media: platform: fsl-viu: add __iomem annotations
Em Fri, 6 Apr 2018 16:23:18 +0200 Arnd Bergmannescreveu: > This avoids countless sparse warnings like > >drivers/media/platform/fsl-viu.c:1081:25: sparse: incorrect type in > argument 2 (different address spaces) >drivers/media/platform/fsl-viu.c:1082:25: sparse: incorrect type in > argument 2 (different address spaces) Heh, that's almost identical to this one: https://git.linuxtv.org/mchehab/experimental.git/commit/?h=compile_test=457312e30430e83f9dc4bf1804acb15b91e5dfc1 > > Signed-off-by: Arnd Bergmann > --- > drivers/media/platform/fsl-viu.c | 26 +- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/platform/fsl-viu.c > b/drivers/media/platform/fsl-viu.c > index 200c47c69a75..cc85620267f1 100644 > --- a/drivers/media/platform/fsl-viu.c > +++ b/drivers/media/platform/fsl-viu.c > @@ -128,7 +128,7 @@ struct viu_dev { > int dma_done; > > /* Hardware register area */ > - struct viu_reg *vr; > + struct viu_reg __iomem *vr; > > /* Interrupt vector */ > int irq; > @@ -244,7 +244,7 @@ struct viu_fmt *format_by_fourcc(int fourcc) > > void viu_start_dma(struct viu_dev *dev) > { > - struct viu_reg *vr = dev->vr; > + struct viu_reg __iomem *vr = dev->vr; > > dev->field = 0; > > @@ -255,7 +255,7 @@ void viu_start_dma(struct viu_dev *dev) > > void viu_stop_dma(struct viu_dev *dev) > { > - struct viu_reg *vr = dev->vr; > + struct viu_reg __iomem *vr = dev->vr; > int cnt = 100; > u32 status_cfg; > > @@ -395,7 +395,7 @@ static void free_buffer(struct videobuf_queue *vq, struct > viu_buf *buf) > > inline int buffer_activate(struct viu_dev *dev, struct viu_buf *buf) > { > - struct viu_reg *vr = dev->vr; > + struct viu_reg __iomem *vr = dev->vr; > int bpp; > > /* setup the DMA base address */ > @@ -703,9 +703,9 @@ static int verify_preview(struct viu_dev *dev, struct > v4l2_window *win) > return 0; > } > > -inline void viu_activate_overlay(struct viu_reg *viu_reg) > +inline void viu_activate_overlay(struct viu_reg __iomem *viu_reg) > { > - struct viu_reg *vr = viu_reg; > + struct viu_reg __iomem *vr = viu_reg; > > out_be32(>field_base_addr, reg_val.field_base_addr); > out_be32(>dma_inc, reg_val.dma_inc); > @@ -985,9 +985,9 @@ inline void viu_activate_next_buf(struct viu_dev *dev, > } > } > > -inline void viu_default_settings(struct viu_reg *viu_reg) > +inline void viu_default_settings(struct viu_reg __iomem *viu_reg) > { > - struct viu_reg *vr = viu_reg; > + struct viu_reg __iomem *vr = viu_reg; > > out_be32(>luminance, 0x9512A254); > out_be32(>chroma_r, 0x0331); > @@ -1001,7 +1001,7 @@ inline void viu_default_settings(struct viu_reg > *viu_reg) > > static void viu_overlay_intr(struct viu_dev *dev, u32 status) > { > - struct viu_reg *vr = dev->vr; > + struct viu_reg __iomem *vr = dev->vr; > > if (status & INT_DMA_END_STATUS) > dev->dma_done = 1; > @@ -1032,7 +1032,7 @@ static void viu_overlay_intr(struct viu_dev *dev, u32 > status) > static void viu_capture_intr(struct viu_dev *dev, u32 status) > { > struct viu_dmaqueue *vidq = >vidq; > - struct viu_reg *vr = dev->vr; > + struct viu_reg __iomem *vr = dev->vr; > struct viu_buf *buf; > int field_num; > int need_two; > @@ -1104,7 +1104,7 @@ static void viu_capture_intr(struct viu_dev *dev, u32 > status) > static irqreturn_t viu_intr(int irq, void *dev_id) > { > struct viu_dev *dev = (struct viu_dev *)dev_id; > - struct viu_reg *vr = dev->vr; > + struct viu_reg __iomem *vr = dev->vr; > u32 status; > u32 error; > > @@ -1169,7 +1169,7 @@ static int viu_open(struct file *file) > struct video_device *vdev = video_devdata(file); > struct viu_dev *dev = video_get_drvdata(vdev); > struct viu_fh *fh; > - struct viu_reg *vr; > + struct viu_reg __iomem *vr; > int minor = vdev->minor; > u32 status_cfg; > > @@ -1305,7 +1305,7 @@ static int viu_release(struct file *file) > return 0; > } > > -void viu_reset(struct viu_reg *reg) > +void viu_reset(struct viu_reg __iomem *reg) > { > out_be32(>status_cfg, 0); > out_be32(>luminance, 0x9512a254); Thanks, Mauro
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Em Fri, 6 Apr 2018 16:16:46 +0200 Arnd Bergmannescreveu: > On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehab > wrote: > > Em Fri, 6 Apr 2018 11:51:16 +0200 > > Arnd Bergmann escreveu: > > > >> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab > >> wrote: > >> > >> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST > >> > > >> > There aren't many things that would be needed to allow it > >> > to build with compile test. > >> > > >> > Add the needed bits. > >> > > >> > Signed-off-by: Mauro Carvalho Chehab > >> > >> Reviewed-by: Arnd Bergmann > > > > Actually, in order to avoid warnings with smatch, the COMPILE_TEST > > macros should be declared as: > > > > +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) > > +#define in_be32(a) ioread32be((void __iomem *)a) > > I would just add the correct annotations, I think they've always been missing. > 2 patches coming in a few minutes. I corrected the annotations too. Now, it gives the same results building for both arm and x86. If you want to double check, the full tree is at: https://git.linuxtv.org/mchehab/experimental.git/log/?h=compile_test > > Arnd Thanks, Mauro
[PATCH 02/21] media: dm365_ipipe: remove an unused var
drivers/staging/media/davinci_vpfe/dm365_ipipe.c:74:17: warning: variable 'dev' set but not used [-Wunused-but-set-variable] struct device *dev; ^~~ Signed-off-by: Mauro Carvalho Chehab--- drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c index 31b9e3011415..dd0cfc79f50c 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c @@ -71,14 +71,12 @@ static int ipipe_set_lutdpc_params(struct vpfe_ipipe_device *ipipe, void *param) { struct vpfe_ipipe_lutdpc *lutdpc = >config.lutdpc; struct vpfe_ipipe_lutdpc *dpc_param; - struct device *dev; if (!param) { memset((void *)lutdpc, 0, sizeof(struct vpfe_ipipe_lutdpc)); goto success; } - dev = ipipe->subdev.v4l2_dev->dev; dpc_param = param; lutdpc->en = dpc_param->en; lutdpc->repl_white = dpc_param->repl_white; -- 2.14.3
[PATCH 03/21] media: davinci_vpfe: fix vpfe_ipipe_init() error handling
As warned: drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1834 vpfe_ipipe_init() error: we previously assumed 'res' could be null (see line 1797) There's something wrong at vpfe_ipipe_init(): 1) it caches the resourse_size() from from the first region and reuses to the second region; 2) the "res" var is overriden 3 times; 3) at free logic, it assumes that "res->start" is not overriden by platform_get_resource(pdev, IORESOURCE_MEM, 6), but that's not true, as it can even be NULL there. This patch fixes the above issues by: a) store the resources used by release_mem_region() on a separate var; b) stop caching resource_size(), using the function where needed. Signed-off-by: Mauro Carvalho Chehab--- drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c index dd0cfc79f50c..b3a193ddd778 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c @@ -1778,25 +1778,25 @@ vpfe_ipipe_init(struct vpfe_ipipe_device *ipipe, struct platform_device *pdev) struct media_pad *pads = >pads[0]; struct v4l2_subdev *sd = >subdev; struct media_entity *me = >entity; - static resource_size_t res_len; - struct resource *res; + struct resource *res, *memres; res = platform_get_resource(pdev, IORESOURCE_MEM, 4); if (!res) return -ENOENT; - res_len = resource_size(res); - res = request_mem_region(res->start, res_len, res->name); - if (!res) + memres = request_mem_region(res->start, resource_size(res), res->name); + if (!memres) return -EBUSY; - ipipe->base_addr = ioremap_nocache(res->start, res_len); + ipipe->base_addr = ioremap_nocache(memres->start, + resource_size(memres)); if (!ipipe->base_addr) goto error_release; res = platform_get_resource(pdev, IORESOURCE_MEM, 6); if (!res) goto error_unmap; - ipipe->isp5_base_addr = ioremap_nocache(res->start, res_len); + ipipe->isp5_base_addr = ioremap_nocache(res->start, + resource_size(res)); if (!ipipe->isp5_base_addr) goto error_unmap; @@ -1831,7 +1831,7 @@ vpfe_ipipe_init(struct vpfe_ipipe_device *ipipe, struct platform_device *pdev) error_unmap: iounmap(ipipe->base_addr); error_release: - release_mem_region(res->start, res_len); + release_mem_region(memres->start, resource_size(memres)); return -ENOMEM; } -- 2.14.3
[PATCH 07/21] media: davinci_vpfe: don't use kernel-doc markup for simple comments
Fix those two warnings: drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c:90: warning: Function parameter or member 'interface' not described in 'MODULE_PARM_DESC' drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c:90: warning: Function parameter or member '(default' not described in 'MODULE_PARM_DESC' Signed-off-by: Mauro Carvalho Chehab--- drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c index 634d38c4bea1..e55c815b9b65 100644 --- a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c +++ b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c @@ -77,7 +77,7 @@ static bool interface; module_param(interface, bool, 0444); module_param(debug, bool, 0644); -/** +/* * VPFE capture can be used for capturing video such as from TVP5146 or TVP7002 * and for capture raw bayer data from camera sensors such as mt9p031. At this * point there is problem in co-existence of mt9p031 and tvp5146 due to i2c -- 2.14.3
[PATCH 01/21] media: davinci_vpfe: remove useless checks from ipipe
The dm365_ipipe_hw.c and dm365_ipipe.c file check if several table pointers, declared at davinci_vpfe_user.h, are filled before using them. The problem is that those pointers come from struct declarations like: struct vpfe_ipipe_yee { ... short table[VPFE_IPIPE_MAX_SIZE_YEE_LUT]; }; So, they can't be NULL! Solve those warnings: drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c:433 ipipe_set_lutdpc_regs() warn: this array is probably non-NULL. 'dpc->table' drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c:763 ipipe_set_gamma_regs() warn: this array is probably non-NULL. 'gamma->table_r' drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c:766 ipipe_set_gamma_regs() warn: this array is probably non-NULL. 'gamma->table_b' drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c:769 ipipe_set_gamma_regs() warn: this array is probably non-NULL. 'gamma->table_g' drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c:791 ipipe_set_3d_lut_regs() warn: this array is probably non-NULL. 'lut_3d->table' drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c:903 ipipe_set_gbce_regs() warn: this array is probably non-NULL. 'gbce->table' drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c:946 ipipe_set_ee_regs() warn: this array is probably non-NULL. 'ee->table' drivers/staging/media/davinci_vpfe/dm365_ipipe.c:59 ipipe_validate_lutdpc_params() warn: this array is probably non-NULL. 'lutdpc->table' drivers/staging/media/davinci_vpfe/dm365_ipipe.c:697 ipipe_get_gamma_params() warn: this array is probably non-NULL. 'gamma_param->table_r' drivers/staging/media/davinci_vpfe/dm365_ipipe.c:705 ipipe_get_gamma_params() warn: this array is probably non-NULL. 'gamma_param->table_g' drivers/staging/media/davinci_vpfe/dm365_ipipe.c:712 ipipe_get_gamma_params() warn: this array is probably non-NULL. 'gamma_param->table_b' drivers/staging/media/davinci_vpfe/dm365_ipipe.c:745 ipipe_get_3d_lut_params() warn: this array is probably non-NULL. 'lut_param->table' drivers/staging/media/davinci_vpfe/dm365_ipipe.c:926 ipipe_get_gbce_params() warn: this array is probably non-NULL. 'gbce_param->table' Signed-off-by: Mauro Carvalho Chehab--- drivers/staging/media/davinci_vpfe/dm365_ipipe.c| 18 -- drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 19 +++ 2 files changed, 7 insertions(+), 30 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c index 6a3434cebd79..31b9e3011415 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c @@ -56,7 +56,7 @@ static int ipipe_validate_lutdpc_params(struct vpfe_ipipe_lutdpc *lutdpc) lutdpc->dpc_size > LUT_DPC_MAX_SIZE) return -EINVAL; - if (lutdpc->en && !lutdpc->table) + if (lutdpc->en) return -EINVAL; for (i = 0; i < lutdpc->dpc_size; i++) @@ -694,7 +694,7 @@ static int ipipe_get_gamma_params(struct vpfe_ipipe_device *ipipe, void *param) table_size = gamma->tbl_size; - if (!gamma->bypass_r && !gamma_param->table_r) { + if (!gamma->bypass_r) { dev_err(dev, "ipipe_get_gamma_params: table ptr empty for R\n"); return -EINVAL; @@ -702,14 +702,14 @@ static int ipipe_get_gamma_params(struct vpfe_ipipe_device *ipipe, void *param) memcpy(gamma_param->table_r, gamma->table_r, (table_size * sizeof(struct vpfe_ipipe_gamma_entry))); - if (!gamma->bypass_g && !gamma_param->table_g) { + if (!gamma->bypass_g) { dev_err(dev, "ipipe_get_gamma_params: table ptr empty for G\n"); return -EINVAL; } memcpy(gamma_param->table_g, gamma->table_g, (table_size * sizeof(struct vpfe_ipipe_gamma_entry))); - if (!gamma->bypass_b && !gamma_param->table_b) { + if (!gamma->bypass_b) { dev_err(dev, "ipipe_get_gamma_params: table ptr empty for B\n"); return -EINVAL; } @@ -739,13 +739,8 @@ static int ipipe_get_3d_lut_params(struct vpfe_ipipe_device *ipipe, void *param) { struct vpfe_ipipe_3d_lut *lut_param = param; struct vpfe_ipipe_3d_lut *lut = >config.lut; - struct device *dev = ipipe->subdev.v4l2_dev->dev; lut_param->en = lut->en; - if (!lut_param->table) { - dev_err(dev, "ipipe_get_3d_lut_params: Invalid table ptr\n"); - return -EINVAL; - } memcpy(lut_param->table, >table, (VPFE_IPIPE_MAX_SIZE_3D_LUT * @@ -919,14 +914,9 @@ static int ipipe_get_gbce_params(struct vpfe_ipipe_device *ipipe, void *param) { struct vpfe_ipipe_gbce *gbce_param = param; struct vpfe_ipipe_gbce *gbce = >config.gbce; - struct device *dev =
[PATCH 06/21] media: davinci_vpfe: vpfe_video: remove an unused var
as warned: drivers/staging/media/davinci_vpfe/vpfe_video.c: In function 'vpfe_streamon': drivers/staging/media/davinci_vpfe/vpfe_video.c:1471:31: warning: variable 'sdinfo' set but not used [-Wunused-but-set-variable] struct vpfe_ext_subdev_info *sdinfo; ^~ While here, cleanup this kernel-doc warning: drivers/staging/media/davinci_vpfe/vpfe_video.c:225: warning: Function parameter or member 'pipe' not described in 'vpfe_video_validate_pipeline' Signed-off-by: Mauro Carvalho Chehab--- drivers/staging/media/davinci_vpfe/vpfe_video.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c index 588743a6fd8a..390fc98d07dd 100644 --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c @@ -214,7 +214,7 @@ int vpfe_video_is_pipe_ready(struct vpfe_pipeline *pipe) return 1; } -/** +/* * Validate a pipeline by checking both ends of all links for format * discrepancies. * @@ -1468,7 +1468,6 @@ static int vpfe_streamon(struct file *file, void *priv, struct vpfe_device *vpfe_dev = video->vpfe_dev; struct vpfe_pipeline *pipe = >pipe; struct vpfe_fh *fh = file->private_data; - struct vpfe_ext_subdev_info *sdinfo; int ret = -EINVAL; v4l2_dbg(1, debug, _dev->v4l2_dev, "vpfe_streamon\n"); @@ -1483,7 +1482,6 @@ static int vpfe_streamon(struct file *file, void *priv, v4l2_err(_dev->v4l2_dev, "fh->io_allowed\n"); return -EACCES; } - sdinfo = video->current_ext_subdev; /* If buffer queue is empty, return error */ if (list_empty(>buffer_queue.queued_list)) { v4l2_err(_dev->v4l2_dev, "buffer queue is empty\n"); -- 2.14.3
[PATCH 05/21] media: davinci_vpfe: get rid of an unused var at dm365_isif.c
Not sure what was the original idea here, but the implementation went into a different way, and the fmt var is not used anymore, as warned: drivers/staging/media/davinci_vpfe/dm365_isif.c: In function '__isif_get_format': drivers/staging/media/davinci_vpfe/dm365_isif.c:1401:29: warning: variable 'fmt' set but not used [-Wunused-but-set-variable] struct v4l2_subdev_format fmt; ^~~ Signed-off-by: Mauro Carvalho Chehab--- drivers/staging/media/davinci_vpfe/dm365_isif.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c b/drivers/staging/media/davinci_vpfe/dm365_isif.c index 569bcdc9ce2f..745e33fa6bea 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_isif.c +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c @@ -1397,14 +1397,9 @@ __isif_get_format(struct vpfe_isif_device *isif, struct v4l2_subdev_pad_config *cfg, unsigned int pad, enum v4l2_subdev_format_whence which) { - if (which == V4L2_SUBDEV_FORMAT_TRY) { - struct v4l2_subdev_format fmt; - - fmt.pad = pad; - fmt.which = which; - + if (which == V4L2_SUBDEV_FORMAT_TRY) return v4l2_subdev_get_try_format(>subdev, cfg, pad); - } + return >formats[pad]; } -- 2.14.3
[PATCH 08/21] media: davinci_vpfe: fix a typo for "default"
resizer_set_defualt_configuration -> resizer_set_default_configuration Signed-off-by: Mauro Carvalho Chehab--- drivers/staging/media/davinci_vpfe/dm365_resizer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c b/drivers/staging/media/davinci_vpfe/dm365_resizer.c index 1ee216d71d42..1e478755fe2f 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c +++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c @@ -794,7 +794,7 @@ resizer_configure_in_single_shot_mode(struct vpfe_resizer_device *resizer) } static void -resizer_set_defualt_configuration(struct vpfe_resizer_device *resizer) +resizer_set_default_configuration(struct vpfe_resizer_device *resizer) { #define WIDTH_I 640 #define HEIGHT_I 480 @@ -916,7 +916,7 @@ resizer_set_configuration(struct vpfe_resizer_device *resizer, struct vpfe_rsz_config *chan_config) { if (!chan_config->config) - resizer_set_defualt_configuration(resizer); + resizer_set_default_configuration(resizer); else if (copy_from_user(>config.user_config, chan_config->config, sizeof(struct vpfe_rsz_config_params))) -- 2.14.3
[PATCH 09/21] media: davinci_vpfe: cleanup ipipe_[g|s]_config logic
Reduce one ident level inside those functions and use BIT() macro. Signed-off-by: Mauro Carvalho Chehab--- drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 98 1 file changed, 50 insertions(+), 48 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c index b3a193ddd778..a7043865cf06 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c @@ -27,6 +27,7 @@ */ #include +#include #include "dm365_ipipe.h" #include "dm365_ipipe_hw.h" @@ -1255,37 +1256,38 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg) int rval = 0; for (i = 0; i < ARRAY_SIZE(ipipe_modules); i++) { - unsigned int bit = 1 << i; + const struct ipipe_module_if *module_if; + struct ipipe_module_params *params; + void __user *from; + size_t size; + void *to; - if (cfg->flag & bit) { - const struct ipipe_module_if *module_if = - _modules[i]; - struct ipipe_module_params *params; - void __user *from = *(void * __user *) - ((void *)cfg + module_if->config_offset); - size_t size; - void *to; + if (!(cfg->flag & BIT(i))) + continue; - params = kmalloc(sizeof(struct ipipe_module_params), -GFP_KERNEL); - to = (void *)params + module_if->param_offset; - size = module_if->param_size; + module_if = _modules[i]; + from = *(void * __user *) + ((void *)cfg + module_if->config_offset); - if (to && from && size) { - if (copy_from_user(to, from, size)) { - rval = -EFAULT; - break; - } - rval = module_if->set(ipipe, to); - if (rval) - goto error; - } else if (to && !from && size) { - rval = module_if->set(ipipe, NULL); - if (rval) - goto error; + params = kmalloc(sizeof(struct ipipe_module_params), + GFP_KERNEL); + to = (void *)params + module_if->param_offset; + size = module_if->param_size; + + if (to && from && size) { + if (copy_from_user(to, from, size)) { + rval = -EFAULT; + break; } - kfree(params); + rval = module_if->set(ipipe, to); + if (rval) + goto error; + } else if (to && !from && size) { + rval = module_if->set(ipipe, NULL); + if (rval) + goto error; } + kfree(params); } error: return rval; @@ -1298,33 +1300,33 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg) int rval = 0; for (i = 1; i < ARRAY_SIZE(ipipe_modules); i++) { - unsigned int bit = 1 << i; + const struct ipipe_module_if *module_if; + struct ipipe_module_params *params; + void __user *to; + size_t size; + void *from; - if (cfg->flag & bit) { - const struct ipipe_module_if *module_if = - _modules[i]; - struct ipipe_module_params *params; - void __user *to = *(void * __user *) - ((void *)cfg + module_if->config_offset); - size_t size; - void *from; + if (!(cfg->flag & BIT(i))) + continue; - params = kmalloc(sizeof(struct ipipe_module_params), - GFP_KERNEL); - from = (void *)params + module_if->param_offset; - size = module_if->param_size; + module_if = _modules[i]; + to = *(void * __user *)((void *)cfg + module_if->config_offset); - if (to && from && size) { - rval = module_if->get(ipipe, from); -
[PATCH 15/21] media: vpbe_display: properly handle error case
if v4l2_subdev_call(..., VENC_GET_FLD,...) fails, it currently returns a random value. Instead, return 1. That's probably better than returning 0, as this is very likely what happens in practice with the current code, as as the probably of an unititialized 32 bits integer to have an specific value (0, in this case), is 1/(2^32). An alternative would be to return an error code, and let the caller to hint, based on the past received frame, but that sounds weird. Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/davinci/vpbe_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c index 9849e4405a6a..a0b5e670d736 100644 --- a/drivers/media/platform/davinci/vpbe_display.c +++ b/drivers/media/platform/davinci/vpbe_display.c @@ -56,8 +56,7 @@ static int vpbe_set_osd_display_params(struct vpbe_display *disp_dev, static int venc_is_second_field(struct vpbe_display *disp_dev) { struct vpbe_device *vpbe_dev = disp_dev->vpbe_dev; - int ret; - int val; + int ret, val; ret = v4l2_subdev_call(vpbe_dev->venc, core, @@ -67,6 +66,7 @@ static int venc_is_second_field(struct vpbe_display *disp_dev) if (ret < 0) { v4l2_err(_dev->v4l2_dev, "Error in getting Field ID 0\n"); + return 1; } return val; } -- 2.14.3
[PATCH 11/21] media: si470x: fix __be16 annotations
The annotations there are wrong as warned: drivers/media/radio/si470x/radio-si470x-i2c.c:107:35: warning: cast to restricted __be16 drivers/media/radio/si470x/radio-si470x-i2c.c:107:35: warning: cast to restricted __be16 drivers/media/radio/si470x/radio-si470x-i2c.c:107:35: warning: cast to restricted __be16 drivers/media/radio/si470x/radio-si470x-i2c.c:107:35: warning: cast to restricted __be16 drivers/media/radio/si470x/radio-si470x-i2c.c:129:24: warning: incorrect type in assignment (different base types) drivers/media/radio/si470x/radio-si470x-i2c.c:129:24:expected unsigned short [unsigned] [short] drivers/media/radio/si470x/radio-si470x-i2c.c:129:24:got restricted __be16 [usertype] drivers/media/radio/si470x/radio-si470x-i2c.c:163:39: warning: cast to restricted __be16 drivers/media/radio/si470x/radio-si470x-i2c.c:163:39: warning: cast to restricted __be16 drivers/media/radio/si470x/radio-si470x-i2c.c:163:39: warning: cast to restricted __be16 drivers/media/radio/si470x/radio-si470x-i2c.c:163:39: warning: cast to restricted __be16 Signed-off-by: Mauro Carvalho Chehab--- drivers/media/radio/si470x/radio-si470x-i2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c index 1b3720b1e737..e3b3ecd14a4d 100644 --- a/drivers/media/radio/si470x/radio-si470x-i2c.c +++ b/drivers/media/radio/si470x/radio-si470x-i2c.c @@ -91,7 +91,7 @@ MODULE_PARM_DESC(max_rds_errors, "RDS maximum block errors: *1*"); */ static int si470x_get_register(struct si470x_device *radio, int regnr) { - u16 buf[READ_REG_NUM]; + __be16 buf[READ_REG_NUM]; struct i2c_msg msgs[1] = { { .addr = radio->client->addr, @@ -116,7 +116,7 @@ static int si470x_get_register(struct si470x_device *radio, int regnr) static int si470x_set_register(struct si470x_device *radio, int regnr) { int i; - u16 buf[WRITE_REG_NUM]; + __be16 buf[WRITE_REG_NUM]; struct i2c_msg msgs[1] = { { .addr = radio->client->addr, @@ -146,7 +146,7 @@ static int si470x_set_register(struct si470x_device *radio, int regnr) static int si470x_get_all_registers(struct si470x_device *radio) { int i; - u16 buf[READ_REG_NUM]; + __be16 buf[READ_REG_NUM]; struct i2c_msg msgs[1] = { { .addr = radio->client->addr, -- 2.14.3
[PATCH 19/21] media: fsl-viu: use %p to print pointers
Solve those warnings: drivers/media/platform/fsl-viu.c:299 restart_video_queue() warn: argument 3 to %08lx specifier is cast from pointer drivers/media/platform/fsl-viu.c:506 buffer_queue() warn: argument 2 to %08lx specifier is cast from pointer drivers/media/platform/fsl-viu.c:518 buffer_queue() warn: argument 2 to %08lx specifier is cast from pointer drivers/media/platform/fsl-viu.c:528 buffer_queue() warn: argument 2 to %08lx specifier is cast from pointer drivers/media/platform/fsl-viu.c:1219 viu_open() warn: argument 2 to %08lx specifier is cast from pointer drivers/media/platform/fsl-viu.c:1219 viu_open() warn: argument 3 to %08lx specifier is cast from pointer drivers/media/platform/fsl-viu.c:1219 viu_open() warn: argument 4 to %08lx specifier is cast from pointer drivers/media/platform/fsl-viu.c:1329 viu_mmap() warn: argument 2 to %08lx specifier is cast from pointer drivers/media/platform/fsl-viu.c:1334 viu_mmap() warn: argument 2 to %08lx specifier is cast from pointer Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/fsl-viu.c | 17 ++--- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c index 6fd1c8f66047..4ca060ee8c08 100644 --- a/drivers/media/platform/fsl-viu.c +++ b/drivers/media/platform/fsl-viu.c @@ -296,7 +296,7 @@ static int restart_video_queue(struct viu_dmaqueue *vidq) { struct viu_buf *buf, *prev; - dprintk(1, "%s vidq=0x%08lx\n", __func__, (unsigned long)vidq); + dprintk(1, "%s vidq=%p\n", __func__, vidq); if (!list_empty(>active)) { buf = list_entry(vidq->active.next, struct viu_buf, vb.queue); dprintk(2, "restart_queue [%p/%d]: restart dma\n", @@ -503,8 +503,7 @@ static void buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb) struct viu_buf *prev; if (!list_empty(>queued)) { - dprintk(1, "adding vb queue=0x%08lx\n", - (unsigned long)>vb.queue); + dprintk(1, "adding vb queue=%p\n", >vb.queue); dprintk(1, "vidq pointer 0x%p, queued 0x%p\n", vidq, >queued); dprintk(1, "dev %p, queued: self %p, next %p, head %p\n", @@ -515,8 +514,7 @@ static void buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb) dprintk(2, "[%p/%d] buffer_queue - append to queued\n", buf, buf->vb.i); } else if (list_empty(>active)) { - dprintk(1, "adding vb active=0x%08lx\n", - (unsigned long)>vb.queue); + dprintk(1, "adding vb active=%p\n", >vb.queue); list_add_tail(>vb.queue, >active); buf->vb.state = VIDEOBUF_ACTIVE; mod_timer(>timeout, jiffies+BUFFER_TIMEOUT); @@ -525,8 +523,7 @@ static void buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb) buffer_activate(dev, buf); } else { - dprintk(1, "adding vb queue2=0x%08lx\n", - (unsigned long)>vb.queue); + dprintk(1, "adding vb queue2=%p\n", >vb.queue); prev = list_entry(vidq->active.prev, struct viu_buf, vb.queue); if (prev->vb.width == buf->vb.width && prev->vb.height == buf->vb.height && @@ -1216,9 +1213,7 @@ static int viu_open(struct file *file) dev->crop_current.width = fh->width; dev->crop_current.height = fh->height; - dprintk(1, "Open: fh=0x%08lx, dev=0x%08lx, dev->vidq=0x%08lx\n", - (unsigned long)fh, (unsigned long)dev, - (unsigned long)>vidq); + dprintk(1, "Open: fh=%p, dev=%p, dev->vidq=%p\n", fh, dev, >vidq); dprintk(1, "Open: list_empty queued=%d\n", list_empty(>vidq.queued)); dprintk(1, "Open: list_empty active=%d\n", @@ -1331,7 +1326,7 @@ static int viu_mmap(struct file *file, struct vm_area_struct *vma) struct viu_dev *dev = fh->dev; int ret; - dprintk(1, "mmap called, vma=0x%08lx\n", (unsigned long)vma); + dprintk(1, "mmap called, vma=%p\n", vma); if (mutex_lock_interruptible(>lock)) return -ERESTARTSYS; -- 2.14.3
[PATCH 18/21] media: isppreview: fix __user annotations
That prevent those warnings: drivers/media/platform/omap3isp/isppreview.c:893:45: warning: incorrect type in initializer (different address spaces) drivers/media/platform/omap3isp/isppreview.c:893:45:expected void [noderef] *from drivers/media/platform/omap3isp/isppreview.c:893:45:got void *[noderef] drivers/media/platform/omap3isp/isppreview.c:893:47: warning: dereference of noderef expression Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/omap3isp/isppreview.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/omap3isp/isppreview.c b/drivers/media/platform/omap3isp/isppreview.c index ac30a0f83780..c2ef5870b231 100644 --- a/drivers/media/platform/omap3isp/isppreview.c +++ b/drivers/media/platform/omap3isp/isppreview.c @@ -890,7 +890,7 @@ static int preview_config(struct isp_prev_device *prev, params = >params.params[!!(active & bit)]; if (cfg->flag & bit) { - void __user *from = *(void * __user *) + void __user *from = *(void __user **) ((void *)cfg + attr->config_offset); void *to = (void *)params + attr->param_offset; size_t size = attr->param_size; -- 2.14.3
[PATCH 1/2] media: platform: fsl-viu: add __iomem annotations
This avoids countless sparse warnings like drivers/media/platform/fsl-viu.c:1081:25: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1082:25: sparse: incorrect type in argument 2 (different address spaces) Signed-off-by: Arnd Bergmann--- drivers/media/platform/fsl-viu.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c index 200c47c69a75..cc85620267f1 100644 --- a/drivers/media/platform/fsl-viu.c +++ b/drivers/media/platform/fsl-viu.c @@ -128,7 +128,7 @@ struct viu_dev { int dma_done; /* Hardware register area */ - struct viu_reg *vr; + struct viu_reg __iomem *vr; /* Interrupt vector */ int irq; @@ -244,7 +244,7 @@ struct viu_fmt *format_by_fourcc(int fourcc) void viu_start_dma(struct viu_dev *dev) { - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; dev->field = 0; @@ -255,7 +255,7 @@ void viu_start_dma(struct viu_dev *dev) void viu_stop_dma(struct viu_dev *dev) { - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; int cnt = 100; u32 status_cfg; @@ -395,7 +395,7 @@ static void free_buffer(struct videobuf_queue *vq, struct viu_buf *buf) inline int buffer_activate(struct viu_dev *dev, struct viu_buf *buf) { - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; int bpp; /* setup the DMA base address */ @@ -703,9 +703,9 @@ static int verify_preview(struct viu_dev *dev, struct v4l2_window *win) return 0; } -inline void viu_activate_overlay(struct viu_reg *viu_reg) +inline void viu_activate_overlay(struct viu_reg __iomem *viu_reg) { - struct viu_reg *vr = viu_reg; + struct viu_reg __iomem *vr = viu_reg; out_be32(>field_base_addr, reg_val.field_base_addr); out_be32(>dma_inc, reg_val.dma_inc); @@ -985,9 +985,9 @@ inline void viu_activate_next_buf(struct viu_dev *dev, } } -inline void viu_default_settings(struct viu_reg *viu_reg) +inline void viu_default_settings(struct viu_reg __iomem *viu_reg) { - struct viu_reg *vr = viu_reg; + struct viu_reg __iomem *vr = viu_reg; out_be32(>luminance, 0x9512A254); out_be32(>chroma_r, 0x0331); @@ -1001,7 +1001,7 @@ inline void viu_default_settings(struct viu_reg *viu_reg) static void viu_overlay_intr(struct viu_dev *dev, u32 status) { - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; if (status & INT_DMA_END_STATUS) dev->dma_done = 1; @@ -1032,7 +1032,7 @@ static void viu_overlay_intr(struct viu_dev *dev, u32 status) static void viu_capture_intr(struct viu_dev *dev, u32 status) { struct viu_dmaqueue *vidq = >vidq; - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; struct viu_buf *buf; int field_num; int need_two; @@ -1104,7 +1104,7 @@ static void viu_capture_intr(struct viu_dev *dev, u32 status) static irqreturn_t viu_intr(int irq, void *dev_id) { struct viu_dev *dev = (struct viu_dev *)dev_id; - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; u32 status; u32 error; @@ -1169,7 +1169,7 @@ static int viu_open(struct file *file) struct video_device *vdev = video_devdata(file); struct viu_dev *dev = video_get_drvdata(vdev); struct viu_fh *fh; - struct viu_reg *vr; + struct viu_reg __iomem *vr; int minor = vdev->minor; u32 status_cfg; @@ -1305,7 +1305,7 @@ static int viu_release(struct file *file) return 0; } -void viu_reset(struct viu_reg *reg) +void viu_reset(struct viu_reg __iomem *reg) { out_be32(>status_cfg, 0); out_be32(>luminance, 0x9512a254); -- 2.9.0
[PATCH 2/2] media: platform: fsl-viu: mark local functions 'static'
Both sparse and gcc (with 'make V=1') warn about non-static symbols that have not been declared: drivers/media/platform/fsl-viu.c:235:16: warning: symbol 'format_by_fourcc' was not declared. Should it be static? drivers/media/platform/fsl-viu.c:248:6: warning: symbol 'viu_start_dma' was not declared. Should it be static? drivers/media/platform/fsl-viu.c:259:6: warning: symbol 'viu_stop_dma' was not declared. Should it be static? drivers/media/platform/fsl-viu.c:808:5: warning: symbol 'vidioc_g_fbuf' was not declared. Should it be static? drivers/media/platform/fsl-viu.c:819:5: warning: symbol 'vidioc_s_fbuf' was not declared. Should it be static? drivers/media/platform/fsl-viu.c:1311:6: warning: symbol 'viu_reset' was not declared. Should it be static? In this driver, all instance should indeed be static. This leads to better optimized code, and avoids potential namespace conflicts. Signed-off-by: Arnd Bergmann--- drivers/media/platform/fsl-viu.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c index cc85620267f1..38c9be51f01f 100644 --- a/drivers/media/platform/fsl-viu.c +++ b/drivers/media/platform/fsl-viu.c @@ -229,7 +229,7 @@ enum status_config { static irqreturn_t viu_intr(int irq, void *dev_id); -struct viu_fmt *format_by_fourcc(int fourcc) +static struct viu_fmt *format_by_fourcc(int fourcc) { int i; @@ -242,7 +242,7 @@ struct viu_fmt *format_by_fourcc(int fourcc) return NULL; } -void viu_start_dma(struct viu_dev *dev) +static void viu_start_dma(struct viu_dev *dev) { struct viu_reg __iomem *vr = dev->vr; @@ -253,7 +253,7 @@ void viu_start_dma(struct viu_dev *dev) out_be32(>status_cfg, INT_FIELD_EN); } -void viu_stop_dma(struct viu_dev *dev) +static void viu_stop_dma(struct viu_dev *dev) { struct viu_reg __iomem *vr = dev->vr; int cnt = 100; @@ -802,7 +802,7 @@ static int vidioc_overlay(struct file *file, void *priv, unsigned int on) return 0; } -int vidioc_g_fbuf(struct file *file, void *priv, struct v4l2_framebuffer *arg) +static int vidioc_g_fbuf(struct file *file, void *priv, struct v4l2_framebuffer *arg) { struct viu_fh *fh = priv; struct viu_dev *dev = fh->dev; @@ -813,7 +813,7 @@ int vidioc_g_fbuf(struct file *file, void *priv, struct v4l2_framebuffer *arg) return 0; } -int vidioc_s_fbuf(struct file *file, void *priv, const struct v4l2_framebuffer *arg) +static int vidioc_s_fbuf(struct file *file, void *priv, const struct v4l2_framebuffer *arg) { struct viu_fh *fh = priv; struct viu_dev *dev = fh->dev; @@ -1305,7 +1305,7 @@ static int viu_release(struct file *file) return 0; } -void viu_reset(struct viu_reg __iomem *reg) +static void viu_reset(struct viu_reg __iomem *reg) { out_be32(>status_cfg, 0); out_be32(>luminance, 0x9512a254); -- 2.9.0
[PATCH 04/21] media: davinci_vpfe: mark __iomem as such
There are several usages of an __iomem memory that aren't marked as such, causing those warnings: drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:201:27: warning: incorrect type in assignment (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:201:27:expected void *ipipeif_base_addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:201:27:got void [noderef] *ipipeif_base_addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:71:27: warning: incorrect type in argument 1 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:71:27:expected void const volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:71:27:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:expected void volatile [noderef] *addr drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:76:26:got void * drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:71:27: warning: incorrect type in argument 1 (different address
[PATCH 10/21] media: davinci_vpfe: fix __user annotations
The __user annotations on this driver are wrong, causing lots of warnings: drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1269:22: warning: incorrect type in assignment (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1269:22:expected void [noderef] *from drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1269:22:got void *[noderef] drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1313:20: warning: incorrect type in assignment (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1313:20:expected void [noderef] *to drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1313:20:got void *[noderef] drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:424:41: warning: incorrect type in initializer (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:424:41:expected struct ipipeif_params *config drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:424:41:got void [noderef] *arg drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:474:46: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:474:46:expected void [noderef] *arg drivers/staging/media/davinci_vpfe/dm365_ipipeif.c:474:46:got void *arg drivers/staging/media/davinci_vpfe/dm365_resizer.c:922:32: warning: incorrect type in argument 2 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_resizer.c:922:32:expected void const [noderef] *from drivers/staging/media/davinci_vpfe/dm365_resizer.c:922:32:got struct vpfe_rsz_config_params *config drivers/staging/media/davinci_vpfe/dm365_resizer.c:945:27: warning: incorrect type in argument 1 (different address spaces) drivers/staging/media/davinci_vpfe/dm365_resizer.c:945:27:expected void [noderef] *to drivers/staging/media/davinci_vpfe/dm365_resizer.c:945:27:got void * Signed-off-by: Mauro Carvalho Chehab--- drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 15 ++- drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 2 +- drivers/staging/media/davinci_vpfe/dm365_resizer.c | 9 + 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c index a7043865cf06..4373e1ea81e6 100644 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c @@ -1258,16 +1258,14 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg) for (i = 0; i < ARRAY_SIZE(ipipe_modules); i++) { const struct ipipe_module_if *module_if; struct ipipe_module_params *params; - void __user *from; + void *from, *to; size_t size; - void *to; if (!(cfg->flag & BIT(i))) continue; module_if = _modules[i]; - from = *(void * __user *) - ((void *)cfg + module_if->config_offset); + from = *(void **)((void *)cfg + module_if->config_offset); params = kmalloc(sizeof(struct ipipe_module_params), GFP_KERNEL); @@ -1275,7 +1273,7 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg) size = module_if->param_size; if (to && from && size) { - if (copy_from_user(to, from, size)) { + if (copy_from_user(to, (void __user *)from, size)) { rval = -EFAULT; break; } @@ -1302,15 +1300,14 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg) for (i = 1; i < ARRAY_SIZE(ipipe_modules); i++) { const struct ipipe_module_if *module_if; struct ipipe_module_params *params; - void __user *to; + void *from, *to; size_t size; - void *from; if (!(cfg->flag & BIT(i))) continue; module_if = _modules[i]; - to = *(void * __user *)((void *)cfg + module_if->config_offset); + to = *(void **)((void *)cfg + module_if->config_offset); params = kmalloc(sizeof(struct ipipe_module_params), GFP_KERNEL); @@ -1321,7 +1318,7 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct vpfe_ipipe_config *cfg) rval = module_if->get(ipipe, from); if (rval) goto error; - if (copy_to_user(to, from, size)) { + if (copy_to_user((void __user *)to, from,
[PATCH 00/21] Fix sparse/smatch errors on non-x86 drivers
After applying this patch series: https://www.mail-archive.com/linux-media@vger.kernel.org/msg128829.html Which allows to build all drivers with COMPILE_TEST on x86, my scripts can finally check for errors/warnings on those drivers. This patch series correct those warnings. Mauro Carvalho Chehab (21): media: davinci_vpfe: remove useless checks from ipipe media: dm365_ipipe: remove an unused var media: davinci_vpfe: fix vpfe_ipipe_init() error handling media: davinci_vpfe: mark __iomem as such media: davinci_vpfe: get rid of an unused var at dm365_isif.c media: davinci_vpfe: vpfe_video: remove an unused var media: davinci_vpfe: don't use kernel-doc markup for simple comments media: davinci_vpfe: fix a typo for "default" media: davinci_vpfe: cleanup ipipe_[g|s]_config logic media: davinci_vpfe: fix __user annotations media: si470x: fix __be16 annotations media: isif: reorder a statement to match coding style media: davinci: fix an inconsistent ident media: mmp-driver: add needed __iomem marks to power_regs media: vpbe_display: properly handle error case media: vpbe_display: get rid of warnings media: ispstat: use %p to print the address of a buffer media: isppreview: fix __user annotations media: fsl-viu: use %p to print pointers media: fsl-viu: fix __iomem annotations media: omap_vout: fix wrong identing drivers/media/platform/davinci/isif.c | 2 +- drivers/media/platform/davinci/vpbe_display.c | 12 +- drivers/media/platform/davinci/vpbe_osd.c | 5 +- drivers/media/platform/fsl-viu.c | 43 +++ drivers/media/platform/marvell-ccic/mmp-driver.c | 2 +- drivers/media/platform/omap/omap_vout.c| 15 ++- drivers/media/platform/omap3isp/isppreview.c | 2 +- drivers/media/platform/omap3isp/ispstat.c | 4 +- drivers/media/radio/si470x/radio-si470x-i2c.c | 6 +- drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 131 ++--- .../staging/media/davinci_vpfe/dm365_ipipe_hw.c| 19 +-- drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 14 +-- drivers/staging/media/davinci_vpfe/dm365_isif.c| 9 +- drivers/staging/media/davinci_vpfe/dm365_resizer.c | 13 +- .../staging/media/davinci_vpfe/vpfe_mc_capture.c | 2 +- drivers/staging/media/davinci_vpfe/vpfe_video.c| 4 +- 16 files changed, 121 insertions(+), 162 deletions(-) -- 2.14.3
[PATCH 16/21] media: vpbe_display: get rid of warnings
Solve those warnings: drivers/media/platform/davinci/vpbe_display.c:288 vpbe_start_streaming() warn: inconsistent indenting drivers/media/platform/davinci/vpbe_display.c:1356 register_device() warn: argument 3 to %x specifier is cast from pointer drivers/media/platform/davinci/vpbe_display.c:1356 register_device() warn: argument 4 to %x specifier is cast from pointer Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/davinci/vpbe_display.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c index a0b5e670d736..b0eb3d899eb4 100644 --- a/drivers/media/platform/davinci/vpbe_display.c +++ b/drivers/media/platform/davinci/vpbe_display.c @@ -285,7 +285,7 @@ static int vpbe_start_streaming(struct vb2_queue *vq, unsigned int count) struct osd_state *osd_device = layer->disp_dev->osd_device; int ret; -osd_device->ops.disable_layer(osd_device, layer->layer_info.id); + osd_device->ops.disable_layer(osd_device, layer->layer_info.id); /* Get the next frame from the buffer queue */ layer->next_frm = layer->cur_frm = list_entry(layer->dma_queue.next, @@ -1354,9 +1354,9 @@ static int register_device(struct vpbe_layer *vpbe_display_layer, v4l2_info(_dev->vpbe_dev->v4l2_dev, "Trying to register VPBE display device.\n"); v4l2_info(_dev->vpbe_dev->v4l2_dev, - "layer=%x,layer->video_dev=%x\n", - (int)vpbe_display_layer, - (int)_display_layer->video_dev); + "layer=%p,layer->video_dev=%p\n", + vpbe_display_layer, + _display_layer->video_dev); vpbe_display_layer->video_dev.queue = _display_layer->buffer_queue; err = video_register_device(_display_layer->video_dev, -- 2.14.3
[PATCH 17/21] media: ispstat: use %p to print the address of a buffer
Instead of converting to int, use %p. That prevents this warning: drivers/media/platform/omap3isp/ispstat.c:451 isp_stat_bufs_alloc() warn: argument 7 to %08lx specifier is cast from pointer Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/omap3isp/ispstat.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c index 47cbc7e3d825..eb1b589b0aeb 100644 --- a/drivers/media/platform/omap3isp/ispstat.c +++ b/drivers/media/platform/omap3isp/ispstat.c @@ -449,10 +449,10 @@ static int isp_stat_bufs_alloc(struct ispstat *stat, u32 size) buf->empty = 1; dev_dbg(stat->isp->dev, - "%s: buffer[%u] allocated. dma=0x%08lx virt=0x%08lx", + "%s: buffer[%u] allocated. dma=0x%08lx virt=%p", stat->subdev.name, i, (unsigned long)buf->dma_addr, - (unsigned long)buf->virt_addr); + buf->virt_addr); } return 0; -- 2.14.3
[PATCH 21/21] media: omap_vout: fix wrong identing
As warned: drivers/media/platform/omap/omap_vout.c:711 omap_vout_buffer_setup() warn: inconsistent indenting Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/omap/omap_vout.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/media/platform/omap/omap_vout.c b/drivers/media/platform/omap/omap_vout.c index a795a9fae899..e2723fedac8d 100644 --- a/drivers/media/platform/omap/omap_vout.c +++ b/drivers/media/platform/omap/omap_vout.c @@ -702,19 +702,18 @@ static int omap_vout_buffer_setup(struct videobuf_queue *q, unsigned int *count, virt_addr = omap_vout_alloc_buffer(vout->buffer_size, _addr); if (!virt_addr) { - if (ovid->rotation_type == VOUT_ROT_NONE) { + if (ovid->rotation_type == VOUT_ROT_NONE) break; - } else { - if (!is_rotation_enabled(vout)) - break; + + if (!is_rotation_enabled(vout)) + break; + /* Free the VRFB buffers if no space for V4L2 buffers */ for (j = i; j < *count; j++) { - omap_vout_free_buffer( - vout->smsshado_virt_addr[j], - vout->smsshado_size); + omap_vout_free_buffer(vout->smsshado_virt_addr[j], + vout->smsshado_size); vout->smsshado_virt_addr[j] = 0; vout->smsshado_phy_addr[j] = 0; - } } } vout->buf_virt_addr[i] = virt_addr; -- 2.14.3
[PATCH 13/21] media: davinci: fix an inconsistent ident
drivers/media/platform/davinci/vpbe_osd.c:849 try_layer_config() warn: inconsistent indenting Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/davinci/vpbe_osd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/davinci/vpbe_osd.c b/drivers/media/platform/davinci/vpbe_osd.c index 99a4ec183ba9..7f610320426d 100644 --- a/drivers/media/platform/davinci/vpbe_osd.c +++ b/drivers/media/platform/davinci/vpbe_osd.c @@ -846,9 +846,10 @@ static int try_layer_config(struct osd_state *sd, enum osd_layer layer, /* DM6446: */ /* only one OSD window at a time can use RGB pixel formats */ - if ((osd->vpbe_type == VPBE_VERSION_1) && - is_osd_win(layer) && is_rgb_pixfmt(lconfig->pixfmt)) { + if ((osd->vpbe_type == VPBE_VERSION_1) && + is_osd_win(layer) && is_rgb_pixfmt(lconfig->pixfmt)) { enum osd_pix_format pixfmt; + if (layer == WIN_OSD0) pixfmt = osd->win[WIN_OSD1].lconfig.pixfmt; else -- 2.14.3
[PATCH 20/21] media: fsl-viu: fix __iomem annotations
Those annotations are wrong, causing this warning: drivers/media/platform/fsl-viu.c:1440:21: warning: incorrect type in assignment (different address spaces) drivers/media/platform/fsl-viu.c:1440:21:expected struct viu_reg *vr drivers/media/platform/fsl-viu.c:1440:21:got struct viu_reg [noderef] *[assigned] viu_regs Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/fsl-viu.c | 26 +++--- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c index 4ca060ee8c08..5b6bfcafc2a4 100644 --- a/drivers/media/platform/fsl-viu.c +++ b/drivers/media/platform/fsl-viu.c @@ -134,7 +134,7 @@ struct viu_dev { int dma_done; /* Hardware register area */ - struct viu_reg *vr; + struct viu_reg __iomem *vr; /* Interrupt vector */ int irq; @@ -250,7 +250,7 @@ static struct viu_fmt *format_by_fourcc(int fourcc) static void viu_start_dma(struct viu_dev *dev) { - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; dev->field = 0; @@ -261,7 +261,7 @@ static void viu_start_dma(struct viu_dev *dev) static void viu_stop_dma(struct viu_dev *dev) { - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; int cnt = 100; u32 status_cfg; @@ -401,7 +401,7 @@ static void free_buffer(struct videobuf_queue *vq, struct viu_buf *buf) inline int buffer_activate(struct viu_dev *dev, struct viu_buf *buf) { - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; int bpp; /* setup the DMA base address */ @@ -706,10 +706,8 @@ static int verify_preview(struct viu_dev *dev, struct v4l2_window *win) return 0; } -inline void viu_activate_overlay(struct viu_reg *viu_reg) +inline void viu_activate_overlay(struct viu_reg __iomem *vr) { - struct viu_reg *vr = viu_reg; - out_be32(>field_base_addr, reg_val.field_base_addr); out_be32(>dma_inc, reg_val.dma_inc); out_be32(>picture_count, reg_val.picture_count); @@ -988,10 +986,8 @@ inline void viu_activate_next_buf(struct viu_dev *dev, } } -inline void viu_default_settings(struct viu_reg *viu_reg) +inline void viu_default_settings(struct viu_reg __iomem *vr) { - struct viu_reg *vr = viu_reg; - out_be32(>luminance, 0x9512A254); out_be32(>chroma_r, 0x0331); out_be32(>chroma_g, 0x06600F38); @@ -1004,7 +1000,7 @@ inline void viu_default_settings(struct viu_reg *viu_reg) static void viu_overlay_intr(struct viu_dev *dev, u32 status) { - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; if (status & INT_DMA_END_STATUS) dev->dma_done = 1; @@ -1035,7 +1031,7 @@ static void viu_overlay_intr(struct viu_dev *dev, u32 status) static void viu_capture_intr(struct viu_dev *dev, u32 status) { struct viu_dmaqueue *vidq = >vidq; - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; struct viu_buf *buf; int field_num; int need_two; @@ -1107,7 +1103,7 @@ static void viu_capture_intr(struct viu_dev *dev, u32 status) static irqreturn_t viu_intr(int irq, void *dev_id) { struct viu_dev *dev = (struct viu_dev *)dev_id; - struct viu_reg *vr = dev->vr; + struct viu_reg __iomem *vr = dev->vr; u32 status; u32 error; @@ -1172,7 +1168,7 @@ static int viu_open(struct file *file) struct video_device *vdev = video_devdata(file); struct viu_dev *dev = video_get_drvdata(vdev); struct viu_fh *fh; - struct viu_reg *vr; + struct viu_reg __iomem *vr; int minor = vdev->minor; u32 status_cfg; @@ -1306,7 +1302,7 @@ static int viu_release(struct file *file) return 0; } -static void viu_reset(struct viu_reg *reg) +static void viu_reset(struct viu_reg __iomem *reg) { out_be32(>status_cfg, 0); out_be32(>luminance, 0x9512a254); -- 2.14.3
[PATCH 14/21] media: mmp-driver: add needed __iomem marks to power_regs
Solve those warnings: drivers/media/platform/marvell-ccic/mmp-driver.c:135:41: warning: incorrect type in argument 2 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:135:41:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:135:41:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:136:44: warning: incorrect type in argument 2 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:136:44:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:136:44:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:174:38: warning: incorrect type in argument 2 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:174:38:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:174:38:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:175:38: warning: incorrect type in argument 2 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:175:38:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:175:38:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:195:48: warning: incorrect type in argument 1 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:195:48:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:195:48:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:196:55: warning: incorrect type in argument 2 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:196:55:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:196:55:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:197:54: warning: incorrect type in argument 2 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:197:54:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:197:54:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:202:48: warning: incorrect type in argument 1 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:202:48:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:202:48:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:203:55: warning: incorrect type in argument 2 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:203:55:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:203:55:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:204:54: warning: incorrect type in argument 2 (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:204:54:expected void [noderef] * drivers/media/platform/marvell-ccic/mmp-driver.c:204:54:got void * drivers/media/platform/marvell-ccic/mmp-driver.c:389:25: warning: incorrect type in assignment (different address spaces) drivers/media/platform/marvell-ccic/mmp-driver.c:389:25:expected void *power_regs drivers/media/platform/marvell-ccic/mmp-driver.c:389:25:got void [noderef] * Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/marvell-ccic/mmp-driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 17d79480e75c..3cf300072348 100644 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -37,7 +37,7 @@ MODULE_LICENSE("GPL"); static char *mcam_clks[] = {"CCICAXICLK", "CCICFUNCLK", "CCICPHYCLK"}; struct mmp_camera { - void *power_regs; + void __iomem *power_regs; struct platform_device *pdev; struct mcam_camera mcam; struct list_head devlist; -- 2.14.3
[PATCH 12/21] media: isif: reorder a statement to match coding style
On all places, we do: void *foo; Here, it is doing, instead: void * foo; That tricks static analyzers, making it see errors where there's none. So, just reorder in order to cleanup those warnings: drivers/media/platform/davinci/isif.c:1066:22: warning: incorrect type in assignment (different address spaces) drivers/media/platform/davinci/isif.c:1066:22:expected void *[noderef] addr drivers/media/platform/davinci/isif.c:1066:22:got void [noderef] * drivers/media/platform/davinci/isif.c:1074:44: warning: incorrect type in assignment (different address spaces) drivers/media/platform/davinci/isif.c:1074:44:expected void [noderef] *static [toplevel] [assigned] base_addr drivers/media/platform/davinci/isif.c:1074:44:got void *[noderef] addr drivers/media/platform/davinci/isif.c:1078:51: warning: incorrect type in assignment (different address spaces) drivers/media/platform/davinci/isif.c:1078:51:expected void [noderef] *static [toplevel] [assigned] linear_tbl0_addr drivers/media/platform/davinci/isif.c:1078:51:got void *[noderef] addr drivers/media/platform/davinci/isif.c:1082:51: warning: incorrect type in assignment (different address spaces) drivers/media/platform/davinci/isif.c:1082:51:expected void [noderef] *static [toplevel] [assigned] linear_tbl1_addr drivers/media/platform/davinci/isif.c:1082:51:got void *[noderef] addr drivers/media/platform/davinci/isif.c:1067:22: warning: dereference of noderef expression Signed-off-by: Mauro Carvalho Chehab--- drivers/media/platform/davinci/isif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/davinci/isif.c b/drivers/media/platform/davinci/isif.c index b14caadcd0df..f924e76e2fbf 100644 --- a/drivers/media/platform/davinci/isif.c +++ b/drivers/media/platform/davinci/isif.c @@ -1027,7 +1027,7 @@ static int isif_probe(struct platform_device *pdev) { void (*setup_pinmux)(void); struct resource *res; - void *__iomem addr; + void __iomem *addr; int status = 0, i; /* Platform data holds setup_pinmux function ptr */ -- 2.14.3
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
On Fri, Apr 6, 2018 at 4:15 PM, Mauro Carvalho Chehabwrote: > Em Fri, 6 Apr 2018 11:51:16 +0200 > Arnd Bergmann escreveu: > >> On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab >> wrote: >> >> > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST >> > >> > There aren't many things that would be needed to allow it >> > to build with compile test. >> > >> > Add the needed bits. >> > >> > Signed-off-by: Mauro Carvalho Chehab >> >> Reviewed-by: Arnd Bergmann > > Actually, in order to avoid warnings with smatch, the COMPILE_TEST > macros should be declared as: > > +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) > +#define in_be32(a) ioread32be((void __iomem *)a) I would just add the correct annotations, I think they've always been missing. 2 patches coming in a few minutes. Arnd
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Em Fri, 6 Apr 2018 11:51:16 +0200 Arnd Bergmannescreveu: > On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehab > wrote: > > > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST > > > > There aren't many things that would be needed to allow it > > to build with compile test. > > > > Add the needed bits. > > > > Signed-off-by: Mauro Carvalho Chehab > > Reviewed-by: Arnd Bergmann Actually, in order to avoid warnings with smatch, the COMPILE_TEST macros should be declared as: +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) +#define in_be32(a) ioread32be((void __iomem *)a) Thanks, Mauro [PATCH] media: fsl-viu: allow building it with COMPILE_TEST There aren't many things that would be needed to allow it to build with compile test. Add the needed bits. Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 03c9dfeb7781..e6eb1eb776e1 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -42,7 +42,7 @@ config VIDEO_SH_VOU config VIDEO_VIU tristate "Freescale VIU Video Driver" - depends on VIDEO_V4L2 && PPC_MPC512x + depends on VIDEO_V4L2 && (PPC_MPC512x || COMPILE_TEST) select VIDEOBUF_DMA_CONTIG default y ---help--- diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c index 9abe79779659..6fd1c8f66047 100644 --- a/drivers/media/platform/fsl-viu.c +++ b/drivers/media/platform/fsl-viu.c @@ -36,6 +36,12 @@ #define DRV_NAME "fsl_viu" #define VIU_VERSION"0.5.1" +/* Allow building this driver with COMPILE_TEST */ +#ifndef CONFIG_PPC +#define out_be32(v, a) iowrite32be(a, (void __iomem *)v) +#define in_be32(a) ioread32be((void __iomem *)a) +#endif + #define BUFFER_TIMEOUT msecs_to_jiffies(500) /* 0.5 seconds */ #defineVIU_VID_MEM_LIMIT 4 /* Video memory limit, in Mb */ @@ -1407,7 +1413,7 @@ static int viu_of_probe(struct platform_device *op) } viu_irq = irq_of_parse_and_map(op->dev.of_node, 0); - if (viu_irq == NO_IRQ) { + if (!viu_irq) { dev_err(>dev, "Error while mapping the irq\n"); return -EINVAL; }
Re: [PATCH v2 05/19] media: fsl-viu: allow building it with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master config: x86_64-allmodconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All warnings (new ones prefixed by >>): drivers/media/platform/fsl-viu.c:1081:25: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1081:25:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1081:25:got unsigned int * drivers/media/platform/fsl-viu.c:1082:25: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1082:25:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1082:25:got unsigned int * drivers/media/platform/fsl-viu.c:1083:25: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1083:25:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1083:25:got unsigned int * drivers/media/platform/fsl-viu.c:1095:17: sparse: incorrect type in argument 1 (different address spaces) drivers/media/platform/fsl-viu.c:1095:17:expected void const volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1095:17:got unsigned int * drivers/media/platform/fsl-viu.c:446:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:446:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:446:9:got unsigned int * drivers/media/platform/fsl-viu.c:447:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:447:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:447:9:got unsigned int * drivers/media/platform/fsl-viu.c:448:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:448:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:448:9:got unsigned int * drivers/media/platform/fsl-viu.c:446:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:446:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:446:9:got unsigned int * drivers/media/platform/fsl-viu.c:447:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:447:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:447:9:got unsigned int * drivers/media/platform/fsl-viu.c:448:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:448:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:448:9:got unsigned int * drivers/media/platform/fsl-viu.c:1000:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1000:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1000:9:got unsigned int * drivers/media/platform/fsl-viu.c:1001:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1001:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1001:9:got unsigned int * drivers/media/platform/fsl-viu.c:1002:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1002:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1002:9:got unsigned int * drivers/media/platform/fsl-viu.c:1003:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1003:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1003:9:got unsigned int * drivers/media/platform/fsl-viu.c:1004:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1004:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1004:9:got unsigned int * drivers/media/platform/fsl-viu.c:1005:9: sparse: incorrect type in argument 2 (different address spaces) drivers/media/platform/fsl-viu.c:1005:9:expected void volatile [noderef] *addr drivers/media/platform/fsl-viu.c:1005:9:got unsigned int * drivers/media/platform/fsl-viu.c:1006:9: sparse: incorrect type in argument 1 (different address spaces) drivers/media/platfo
Re: [PATCH v2 02/19] media: omap3isp: allow it to build with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master config: x86_64-allmodconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All warnings (new ones prefixed by >>): drivers/media//platform/omap3isp/ispccdc.c: In function 'ccdc_config': >> drivers/media//platform/omap3isp/ispccdc.c:738:9: warning: cast to pointer >> from integer of different size [-Wint-to-pointer-cast] (__force void __user *)fpc.fpcaddr, ^ sparse warnings: (new ones prefixed by >>) >> drivers/media/platform/omap3isp/isppreview.c:893:45: sparse: incorrect type >> in initializer (different address spaces) @@expected void [noderef] >> *from @@got void [noderef] *from @@ drivers/media/platform/omap3isp/isppreview.c:893:45:expected void [noderef] *from drivers/media/platform/omap3isp/isppreview.c:893:45:got void *[noderef] >> drivers/media/platform/omap3isp/isppreview.c:893:47: sparse: dereference of >> noderef expression vim +738 drivers/media//platform/omap3isp/ispccdc.c de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 656 de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 657 /* de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 658 * ccdc_config - Set CCDC configuration from userspace de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 659 * @ccdc: Pointer to ISP CCDC device. 872aba51 drivers/media/platform/omap3isp/ispccdc.c Lad, Prabhakar 2014-02-21 660 * @ccdc_struct: Structure containing CCDC configuration sent from userspace. de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 661 * de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 662 * Returns 0 if successful, -EINVAL if the pointer to the configuration de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 663 * structure is null, or the copy_from_user function fails to copy user space de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 664 * memory to kernel space memory. de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 665 */ de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 666 static int ccdc_config(struct isp_ccdc_device *ccdc, de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 667struct omap3isp_ccdc_update_config *ccdc_struct) de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 668 { de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 669 struct isp_device *isp = to_isp_device(ccdc); de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 670 unsigned long flags; de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 671 de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 672 spin_lock_irqsave(>lock, flags); de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 673 ccdc->shadow_update = 1; de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 674 spin_unlock_irqrestore(>lock, flags); de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 675 de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 676 if (OMAP3ISP_CCDC_ALAW & ccdc_struct->update) { de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 677 ccdc->alaw = !!(OMAP3ISP_CCDC_ALAW & ccdc_struct->flag); de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 678 ccdc->update |= OMAP3ISP_CCDC_ALAW; de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 679 } de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 680 de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 681 if (OMAP3ISP_CCDC_LPF & ccdc_struct->update) { de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 682 ccdc->lpf = !!(OMAP3ISP_CCDC_LPF & ccdc_struct->flag); de1135d4 drivers/media/video/omap3isp/ispccdc.cLaurent Pinchart 2011-02-12 683 ccdc-&g
Re: [RfC PATCH] Add udmabuf misc device
On 04/06/2018 02:57 PM, Gerd Hoffmann wrote: Hi, I fail to see any common ground for xen-zcopy and udmabuf ... Does the above mean you can assume that xen-zcopy and udmabuf can co-exist as two different solutions? Well, udmabuf route isn't fully clear yet, but yes. See also gvt (intel vgpu), where the hypervisor interface is abstracted away into a separate kernel modules even though most of the actual vgpu emulation code is common. Thank you for your input, I'm just trying to figure out which of the three z-copy solutions intersect and how much And what about hyper-dmabuf? No idea, didn't look at it in detail. Looks pretty complex from a distant view. Maybe because it tries to build a communication framework using dma-bufs instead of a simple dma-buf passing mechanism. Yes, I am looking at it now, trying to figure out the full story and its implementation. BTW, Intel guys were about to share some test application for hyper-dmabuf, maybe I have missed one. It could probably better explain the use-cases and the complexity they have in hyper-dmabuf. Like xen-zcopy it seems to depend on the idea that the hypervisor manages all memory it is easy for guests to share pages with the help of the hypervisor. So, for xen-zcopy we were not trying to make it generic, it just solves display (dumb) zero-copying use-cases for Xen. We implemented it as a DRM helper driver because we can't see any other use-cases as of now. For example, we also have Xen para-virtualized sound driver, but its buffer memory usage is not comparable to what display wants and it works somewhat differently (e.g. there is no "frame done" event, so one can't tell when the sound buffer can be "flipped"). At the same time, we do not use virtio-gpu, so this could probably be one more candidate for shared dma-bufs some day. Which simply isn't the case on kvm. hyper-dmabuf and xen-zcopy could maybe share code, or hyper-dmabuf build on top of xen-zcopy. Hm, I can imagine that: xen-zcopy could be a library code for hyper-dmabuf in terms of implementing all that page sharing fun in multiple directions, e.g. Host->Guest, Guest->Host, Guest<->Guest. But I'll let Matt and Dongwon to comment on that. cheers, Gerd Thank you, Oleksandr P.S. Sorry for making your original mail thread to discuss things much broader than your RFC...
Re: [PATCH v2] Add udmabuf misc device
Am 06.04.2018 um 11:33 schrieb Gerd Hoffmann: Hi, The pages backing a DMA-buf are not allowed to move (at least not without a patch set I'm currently working on), but for certain MM operations to work correctly you must be able to modify the page tables entries and move the pages backing them around. For example try to use fork() with some copy on write pages with this approach. You will find that you have only two options to correctly handle this. The fork() issue should go away with shared memory pages (no cow). I guess this is the reason why vgem is internally backed by shmem. Yes, exactly that is also an approach which should work fine. Just don't try to get this working with get_user_pages(). Hmm. So I could try to limit the udmabuf driver to shmem too (i.e. have the ioctl take a shmem filehandle and offset instead of a virtual address). But maybe it is better then to just extend vgem, i.e. add support to create gem objects from existing shmem. Comments? Yes, extending vgem instead of creating something new sounds like a good idea to me as well. Regards, Christian. cheers, Gerd
Re: [PATCH v2 16/19] media: omap: allow building it with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sparc64 All warnings (new ones prefixed by >>): In file included from arch/sparc/include/asm/page.h:8:0, from arch/sparc/include/asm/thread_info_64.h:27, from arch/sparc/include/asm/thread_info.h:5, from include/linux/thread_info.h:38, from include/asm-generic/preempt.h:5, from ./arch/sparc/include/generated/asm/preempt.h:1, from include/linux/preempt.h:81, from include/linux/spinlock.h:51, from include/linux/seqlock.h:36, from include/linux/time.h:6, from include/linux/stat.h:19, from include/linux/module.h:10, from drivers/media/platform/omap/omap_vout.c:33: drivers/media/platform/omap/omap_vout.c: In function 'omap_vout_get_userptr': drivers/media/platform/omap/omap_vout.c:209:25: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] *physp = virt_to_phys((void *)virtp); ^ arch/sparc/include/asm/page_64.h:147:36: note: in definition of macro '__pa' #define __pa(x) ((unsigned long)(x) - PAGE_OFFSET) ^ >> drivers/media/platform/omap/omap_vout.c:209:12: note: in expansion of macro >> 'virt_to_phys' *physp = virt_to_phys((void *)virtp); ^~~~ vim +/virt_to_phys +209 drivers/media/platform/omap/omap_vout.c 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 @33 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 34 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 35 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 36 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 37 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 38 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 39 #include 72915e85 drivers/media/video/omap/omap_vout.cAmber Jain 2011-07-07 40 #include d1ee8878 drivers/media/video/omap/omap_vout.cGary Thomas 2011-12-01 41 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 42 dd880dd4 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-05-27 43 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 44 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 45 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 46 6a1c9f6d drivers/media/platform/omap/omap_vout.c Tomi Valkeinen 2012-10-08 47 #include 781a1622 drivers/media/platform/omap/omap_vout.c Peter Ujfalusi 2016-05-27 48 #include 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 49 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 50 #include "omap_voutlib.h" 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 51 #include "omap_voutdef.h" 445e258f drivers/media/video/omap/omap_vout.cArchit Taneja2011-06-14 52 #include "omap_vout_vrfb.h" 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 53 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 54 MODULE_AUTHOR("Texas Instruments"); 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 55 MODULE_DESCRIPTION("OMAP Video for Linux Video out driver"); 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 56 MODULE_LICENSE("GPL"); 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 57 5c7ab634 drivers/media/video/omap/omap_vout.cVaibhav Hiremath 2010-04-11 58 /* Driver Configuration macros */ 5c7ab634 drivers/media/video/omap/omap_vout.cVaibha
Re: [RfC PATCH] Add udmabuf misc device
Hi, > > I fail to see any common ground for xen-zcopy and udmabuf ... > Does the above mean you can assume that xen-zcopy and udmabuf > can co-exist as two different solutions? Well, udmabuf route isn't fully clear yet, but yes. See also gvt (intel vgpu), where the hypervisor interface is abstracted away into a separate kernel modules even though most of the actual vgpu emulation code is common. > And what about hyper-dmabuf? No idea, didn't look at it in detail. Looks pretty complex from a distant view. Maybe because it tries to build a communication framework using dma-bufs instead of a simple dma-buf passing mechanism. Like xen-zcopy it seems to depend on the idea that the hypervisor manages all memory it is easy for guests to share pages with the help of the hypervisor. Which simply isn't the case on kvm. hyper-dmabuf and xen-zcopy could maybe share code, or hyper-dmabuf build on top of xen-zcopy. cheers, Gerd
Re: [PATCH v2 15/19] omap2: omapfb: allow building it with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master config: mips-allmodconfig (attached as .config) compiler: mips-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=mips All warnings (new ones prefixed by >>): drivers/video/fbdev/omap2/omapfb/dss/dispc.c: In function 'pixinc': >> drivers/video/fbdev/omap2/omapfb/dss/dispc.c:1859:2: warning: this 'else' >> clause does not guard... [-Wmisleading-indentation] else ^~~~ drivers/video/fbdev/omap2/omapfb/dss/dispc.c:1861:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'else' return 0; ^~ vim +/else +1859 drivers/video/fbdev/omap2/omapfb/dss/dispc.c f76ee892 Tomi Valkeinen 2015-12-09 1850 f76ee892 Tomi Valkeinen 2015-12-09 1851 static s32 pixinc(int pixels, u8 ps) f76ee892 Tomi Valkeinen 2015-12-09 1852 { f76ee892 Tomi Valkeinen 2015-12-09 1853if (pixels == 1) f76ee892 Tomi Valkeinen 2015-12-09 1854return 1; f76ee892 Tomi Valkeinen 2015-12-09 1855else if (pixels > 1) f76ee892 Tomi Valkeinen 2015-12-09 1856return 1 + (pixels - 1) * ps; f76ee892 Tomi Valkeinen 2015-12-09 1857else if (pixels < 0) f76ee892 Tomi Valkeinen 2015-12-09 1858return 1 - (-pixels + 1) * ps; f76ee892 Tomi Valkeinen 2015-12-09 @1859else f76ee892 Tomi Valkeinen 2015-12-09 1860BUG(); f76ee892 Tomi Valkeinen 2015-12-09 1861return 0; f76ee892 Tomi Valkeinen 2015-12-09 1862 } f76ee892 Tomi Valkeinen 2015-12-09 1863 :: The code at line 1859 was first introduced by commit :: f76ee892a99e68b55402b8d4b8aeffcae2aff34d omapfb: copy omapdss & displays for omapfb :: TO: Tomi Valkeinen <tomi.valkei...@ti.com> :: CC: Tomi Valkeinen <tomi.valkei...@ti.com> --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [RfC PATCH] Add udmabuf misc device
On 04/06/2018 12:07 PM, Gerd Hoffmann wrote: I'm not sure we can create something which works on both kvm and xen. The memory management model is quite different ... On xen the hypervisor manages all memory. Guests can allow other guests to access specific pages (using grant tables). In theory any guest <=> guest communication is possible. In practice is mostly guest <=> dom0 because guests access their virtual hardware that way. dom0 is the priviledged guest which owns any hardware not managed by xen itself. Xen guests can ask the hypervisor to update the mapping of guest physical pages. They can ballon down (unmap and free pages). They can ballon up (ask the hypervisor to map fresh pages). They can map pages exported by other guests using grant tables. xen-zcopy makes heavy use of this. It balloons down, to make room in the guest physical address space, then goes map the exported pages there, finally composes a dma-buf. On kvm qemu manages all guest memory. qemu also has all guest memory mapped, so a grant-table like mechanism isn't needed to implement virtual devices. qemu can decide how it backs memory for the guest. qemu propagates the guest memory map to the kvm driver in the linux kernel. kvm guests have some control over the guest memory map, for example they can map pci bars wherever they want in their guest physical address space by programming the base registers accordingly, but unlike xen guests they can't ask the host to remap individual pages. Due to qemu having all guest memory mapped virtual devices are typically designed to have the guest allocate resources, then notify the host where they are located. This is where the udmabuf idea comes from: Guest tells the host (qemu) where the gem object is, and qemu then can create a dmabuf backed by those pages to pass it on to other processes such as the wayland display server. Possibly even without the guest explicitly asking for it, i.e. export the framebuffer placed by the guest in the (virtual) vga pci memory bar as dma-buf. And I can imagine that this is useful outsize virtualization too. I fail to see any common ground for xen-zcopy and udmabuf ... Does the above mean you can assume that xen-zcopy and udmabuf can co-exist as two different solutions? And what about hyper-dmabuf? Thank you, Oleksandr
Re: [RfC PATCH] Add udmabuf misc device
On Fri, Apr 06, 2018 at 10:52:21AM +0100, Daniel Stone wrote: > Hi Gerd, > > On 14 March 2018 at 08:03, Gerd Hoffmannwrote: > >> Either mlock account (because it's mlocked defacto), and get_user_pages > >> won't do that for you. > >> > >> Or you write the full-blown userptr implementation, including mmu_notifier > >> support (see i915 or amdgpu), but that also requires Christian Königs > >> latest ->invalidate_mapping RFC for dma-buf (since atm exporting userptr > >> buffers is a no-go). > > > > I guess I'll look at mlock accounting for starters then. Easier for > > now, and leaves the door open to switch to userptr later as this should > > be transparent to userspace. > > Out of interest, do you have usecases for full userptr support? Maybe > another way would be to allow creation of dmabufs from memfds. I have two things in mind. One is vga emulation. I have virtual pci memory bar for the virtual vga. qemu backs vga memory with anonymous pages right now, switching that to shmem should be easy though if that makes things easier. Guest places the framebuffer somewhere in the pci bar, and I want export the chunk which represents the framebuffer as dma-buf to display it on the host without copying around data. Framebuffer is linear in guest physical memory, so a single block only. That is the simpler case. The more difficuilt one is virtio-gpu ressources. virtio-gpu resources live in host memory (guest has no direct access). The guest can optionally specify guest memory pages as backing storage for the resource. Guest backing storage is allowed to be scattered. Commands exist to copy both ways between host storage and guest backing. With virgl (opengl acceleration) enabled the guest will send rendering commands to fill the framebuffer ressource, so there is no need to copy content to the framebuffer ressource. The guest may fill other resources such as textures used for rendering with copy commands. Without acceleration the guest does software-rendering to the backing storage, then sends a command to copy the framebuffer content from guest backing storage to host ressource. Now it would be useful to allow a shared mapping, so no copying between guest backing storage and host resource is needed, especially for the software rendering case (i.e. dumb gem buffers). Being able to export guest dumb buffers to other host processes would be useful too, for example to display guest windows seamlessly on the host wayland server. So getting a dma-buf for the guest backing storage via udmabuf looked like a useful approach. We can export the guest gem buffers to other host processes that way. qemu itself could map it too, to get a linear representation of the scattered guest backing storage. The other obvious approach would be to do it the other way around and allow the guest map the host resource somehow. On the host side qemu could use vgem to allocate resource memory, so it'll be a gem object already. Mapping that into the guest isn't that straight-forward though. The guest manages its physical address space, so the guest would need to find a free spot and ask the host to place the resource there. Then the guest needs page structs covering the mapped resource, so it can work with it. Didn't investigate how difficuilt that is. Use memory hotplug maybe? Can we easily unmap the resource then? Also I think updating the guests physical memory layout (which we would need to do on every resource map/unmap) isn't an exactly cheap operation ... cheers, Gerd
Re: [PATCH v2 05/19] media: fsl-viu: allow building it with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sparc64 All warnings (new ones prefixed by >>): >> drivers/media/platform/fsl-viu.c:41:0: warning: "NO_IRQ" redefined #define NO_IRQ 0 In file included from arch/sparc/include/asm/irq.h:5:0, from include/linux/interrupt.h:19, from drivers/media/platform/fsl-viu.c:22: arch/sparc/include/asm/irq_64.h:98:0: note: this is the location of the previous definition #define NO_IRQ 0x drivers/media/platform/fsl-viu.c: In function 'viu_setup_preview': drivers/media/platform/fsl-viu.c:760:28: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] reg_val.field_base_addr = (u32)dev->ovbuf.base; ^ vim +/NO_IRQ +41 drivers/media/platform/fsl-viu.c 38 39 /* Allow building this driver with COMPILE_TEST */ 40 #ifndef CONFIG_PPC_MPC512x > 41 #define NO_IRQ 0 42 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [RfC PATCH] Add udmabuf misc device
Hi Gerd, On 14 March 2018 at 08:03, Gerd Hoffmannwrote: >> Either mlock account (because it's mlocked defacto), and get_user_pages >> won't do that for you. >> >> Or you write the full-blown userptr implementation, including mmu_notifier >> support (see i915 or amdgpu), but that also requires Christian Königs >> latest ->invalidate_mapping RFC for dma-buf (since atm exporting userptr >> buffers is a no-go). > > I guess I'll look at mlock accounting for starters then. Easier for > now, and leaves the door open to switch to userptr later as this should > be transparent to userspace. Out of interest, do you have usecases for full userptr support? Maybe another way would be to allow creation of dmabufs from memfds. Cheers, Daniel
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
On Fri, Apr 6, 2018 at 11:47 AM, Mauro Carvalho Chehabwrote: > [PATCH] media: fsl-viu: allow building it with COMPILE_TEST > > There aren't many things that would be needed to allow it > to build with compile test. > > Add the needed bits. > > Signed-off-by: Mauro Carvalho Chehab Reviewed-by: Arnd Bergmann
Re: [PATCH 05/16] media: fsl-viu: allow building it with COMPILE_TEST
Em Thu, 5 Apr 2018 23:35:06 +0200 Arnd Bergmannescreveu: > On Thu, Apr 5, 2018 at 7:54 PM, Mauro Carvalho Chehab > wrote: > > There aren't many things that would be needed to allow it > > to build with compile test. > > > +/* Allow building this driver with COMPILE_TEST */ > > +#ifndef CONFIG_PPC_MPC512x > > +#define NO_IRQ 0 > > The NO_IRQ usage here really needs to die. The portable way to do this > is the simpler > > diff --git a/drivers/media/platform/fsl-viu.c > b/drivers/media/platform/fsl-viu.c > index 200c47c69a75..707bda89b4f7 100644 > --- a/drivers/media/platform/fsl-viu.c > +++ b/drivers/media/platform/fsl-viu.c > @@ -1407,7 +1407,7 @@ static int viu_of_probe(struct platform_device *op) > } > > viu_irq = irq_of_parse_and_map(op->dev.of_node, 0); > - if (viu_irq == NO_IRQ) { > + if (!viu_irq) { > dev_err(>dev, "Error while mapping the irq\n"); > return -EINVAL; > } > > > +#define out_be32(v, a) writel(a, v) > > +#define in_be32(a) readl(a) > > This does get it to compile, but looks confusing because it mixes up the > endianess. I'd suggest doing it like > > #ifndef CONFIG_PPC > #define out_be32(v, a) iowrite32be(a, v) > #define in_be32(a) ioread32be(a) > #endif > > Arnd Thanks for the review. Yeah, that looks better. Patch enclosed. Thanks, Mauro [PATCH] media: fsl-viu: allow building it with COMPILE_TEST There aren't many things that would be needed to allow it to build with compile test. Add the needed bits. Signed-off-by: Mauro Carvalho Chehab diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 03c9dfeb7781..e6eb1eb776e1 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -42,7 +42,7 @@ config VIDEO_SH_VOU config VIDEO_VIU tristate "Freescale VIU Video Driver" - depends on VIDEO_V4L2 && PPC_MPC512x + depends on VIDEO_V4L2 && (PPC_MPC512x || COMPILE_TEST) select VIDEOBUF_DMA_CONTIG default y ---help--- diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c index 9abe79779659..f54592c431d3 100644 --- a/drivers/media/platform/fsl-viu.c +++ b/drivers/media/platform/fsl-viu.c @@ -36,6 +36,12 @@ #define DRV_NAME "fsl_viu" #define VIU_VERSION"0.5.1" +/* Allow building this driver with COMPILE_TEST */ +#ifndef CONFIG_PPC +#define out_be32(v, a) iowrite32be(a, v) +#define in_be32(a) ioread32be(a) +#endif + #define BUFFER_TIMEOUT msecs_to_jiffies(500) /* 0.5 seconds */ #defineVIU_VID_MEM_LIMIT 4 /* Video memory limit, in Mb */ @@ -1407,7 +1413,7 @@ static int viu_of_probe(struct platform_device *op) } viu_irq = irq_of_parse_and_map(op->dev.of_node, 0); - if (viu_irq == NO_IRQ) { + if (!viu_irq) { dev_err(>dev, "Error while mapping the irq\n"); return -EINVAL; }
Re: [RfC PATCH] Add udmabuf misc device
On 04/06/2018 12:07 PM, Gerd Hoffmann wrote: Hi, * The general interface should be able to express sharing from any guest:guest, not just guest:host. Arbitrary G:G sharing might be something some hypervisors simply aren't able to support, but the userspace API itself shouldn't make assumptions or restrict that. I think ideally the sharing API would include some kind of query_targets interface that would return a list of VM's that your current OS is allowed to share with; that list would be depend on the policy established by the system integrator, but obviously wouldn't include targets that the hypervisor itself wouldn't be capable of handling. Can you give a use-case for this? I mean that the system integrator is the one who defines which guests/hosts talk to each other, but querying means that it is possible that VMs have some sort of discovery mechanism, so they can decide on their own whom to connect to. Note that vsock (created by vmware, these days also has a virtio transport for kvm) started with support for both guest <=> host and guest <=> guest support. But later on guest <=> guest was dropped. As far I know the reasons where (a) lack of use cases and (b) security. So, I likewise would know more details on the use cases you have in mind here. Unless we have a compelling use case here I'd suggest to drop the guest <=> guest requirement as it makes the whole thing alot more complex. This is exactly the use-case we have: in our setup Dom0 doesn't own any HW at all and all the HW is passed into a dedicated driver domain (DomD) which is still a guest domain. Then, buffers are shared between two guests, for example, DomD and DomA (Android guest) * The sharing API could be used to share multiple kinds of content in a single system. The sharing sink driver running in the content producer's VM should accept some additional metadata that will be passed over to the target VM as well. Not sure this should be part of hyper-dmabuf. A dma-buf is nothing but a block of data, period. Therefore protocols with dma-buf support (wayland for example) typically already send over metadata describing the content, so duplicating that in hyper-dmabuf looks pointless. 1. We are targeting ARM and one of the major requirements for the buffer sharing is the ability to allocate physically contiguous buffers, which gets even more complicated for systems not backed with an IOMMU. So, for some use-cases it is enough to make the buffers contiguous in terms of IPA and sometimes those need to be contiguous in terms of PA. Which pretty much implies the host must to the allocation. 2. For Xen we would love to see UAPI to create a dma-buf from grant references provided, so we can use this generic solution to implement zero-copying without breaking the existing Xen protocols. This can probably be extended to other hypervizors as well. I'm not sure we can create something which works on both kvm and xen. The memory management model is quite different ... On xen the hypervisor manages all memory. Guests can allow other guests to access specific pages (using grant tables). In theory any guest <=> guest communication is possible. In practice is mostly guest <=> dom0 because guests access their virtual hardware that way. dom0 is the priviledged guest which owns any hardware not managed by xen itself. Please see above for our setup with DomD and Dom0 being a generic ARMv8 domain, no HW Xen guests can ask the hypervisor to update the mapping of guest physical pages. They can ballon down (unmap and free pages). They can ballon up (ask the hypervisor to map fresh pages). They can map pages exported by other guests using grant tables. xen-zcopy makes heavy use of this. It balloons down, to make room in the guest physical address space, then goes map the exported pages there, finally composes a dma-buf. This is what it does On kvm qemu manages all guest memory. qemu also has all guest memory mapped, so a grant-table like mechanism isn't needed to implement virtual devices. qemu can decide how it backs memory for the guest. qemu propagates the guest memory map to the kvm driver in the linux kernel. kvm guests have some control over the guest memory map, for example they can map pci bars wherever they want in their guest physical address space by programming the base registers accordingly, but unlike xen guests they can't ask the host to remap individual pages. Due to qemu having all guest memory mapped virtual devices are typically designed to have the guest allocate resources, then notify the host where they are located. This is where the udmabuf idea comes from: Guest tells the host (qemu) where the gem object is, and qemu then can create a dmabuf backed by those pages to pass it on to other processes such as the wayland display server. Possibly even without the guest explicitly asking for it, i.e. export the framebuffer placed by the
Re: [PATCH v2] Add udmabuf misc device
Hi, > The pages backing a DMA-buf are not allowed to move (at least not without a > patch set I'm currently working on), but for certain MM operations to work > correctly you must be able to modify the page tables entries and move the > pages backing them around. > > For example try to use fork() with some copy on write pages with this > approach. You will find that you have only two options to correctly handle > this. The fork() issue should go away with shared memory pages (no cow). I guess this is the reason why vgem is internally backed by shmem. Hmm. So I could try to limit the udmabuf driver to shmem too (i.e. have the ioctl take a shmem filehandle and offset instead of a virtual address). But maybe it is better then to just extend vgem, i.e. add support to create gem objects from existing shmem. Comments? cheers, Gerd
Re: [RfC PATCH] Add udmabuf misc device
Hi, > > * The general interface should be able to express sharing from any > > guest:guest, not just guest:host. Arbitrary G:G sharing might be > > something some hypervisors simply aren't able to support, but the > > userspace API itself shouldn't make assumptions or restrict that. I > > think ideally the sharing API would include some kind of > > query_targets interface that would return a list of VM's that your > > current OS is allowed to share with; that list would be depend on the > > policy established by the system integrator, but obviously wouldn't > > include targets that the hypervisor itself wouldn't be capable of > > handling. > Can you give a use-case for this? I mean that the system integrator > is the one who defines which guests/hosts talk to each other, > but querying means that it is possible that VMs have some sort > of discovery mechanism, so they can decide on their own whom > to connect to. Note that vsock (created by vmware, these days also has a virtio transport for kvm) started with support for both guest <=> host and guest <=> guest support. But later on guest <=> guest was dropped. As far I know the reasons where (a) lack of use cases and (b) security. So, I likewise would know more details on the use cases you have in mind here. Unless we have a compelling use case here I'd suggest to drop the guest <=> guest requirement as it makes the whole thing alot more complex. > > * The sharing API could be used to share multiple kinds of content in a > > single system. The sharing sink driver running in the content > > producer's VM should accept some additional metadata that will be > > passed over to the target VM as well. Not sure this should be part of hyper-dmabuf. A dma-buf is nothing but a block of data, period. Therefore protocols with dma-buf support (wayland for example) typically already send over metadata describing the content, so duplicating that in hyper-dmabuf looks pointless. > 1. We are targeting ARM and one of the major requirements for the buffer > sharing is the ability to allocate physically contiguous buffers, which gets > even more complicated for systems not backed with an IOMMU. So, for some > use-cases it is enough to make the buffers contiguous in terms of IPA and > sometimes those need to be contiguous in terms of PA. Which pretty much implies the host must to the allocation. > 2. For Xen we would love to see UAPI to create a dma-buf from grant > references provided, so we can use this generic solution to implement > zero-copying without breaking the existing Xen protocols. This can > probably be extended to other hypervizors as well. I'm not sure we can create something which works on both kvm and xen. The memory management model is quite different ... On xen the hypervisor manages all memory. Guests can allow other guests to access specific pages (using grant tables). In theory any guest <=> guest communication is possible. In practice is mostly guest <=> dom0 because guests access their virtual hardware that way. dom0 is the priviledged guest which owns any hardware not managed by xen itself. Xen guests can ask the hypervisor to update the mapping of guest physical pages. They can ballon down (unmap and free pages). They can ballon up (ask the hypervisor to map fresh pages). They can map pages exported by other guests using grant tables. xen-zcopy makes heavy use of this. It balloons down, to make room in the guest physical address space, then goes map the exported pages there, finally composes a dma-buf. On kvm qemu manages all guest memory. qemu also has all guest memory mapped, so a grant-table like mechanism isn't needed to implement virtual devices. qemu can decide how it backs memory for the guest. qemu propagates the guest memory map to the kvm driver in the linux kernel. kvm guests have some control over the guest memory map, for example they can map pci bars wherever they want in their guest physical address space by programming the base registers accordingly, but unlike xen guests they can't ask the host to remap individual pages. Due to qemu having all guest memory mapped virtual devices are typically designed to have the guest allocate resources, then notify the host where they are located. This is where the udmabuf idea comes from: Guest tells the host (qemu) where the gem object is, and qemu then can create a dmabuf backed by those pages to pass it on to other processes such as the wayland display server. Possibly even without the guest explicitly asking for it, i.e. export the framebuffer placed by the guest in the (virtual) vga pci memory bar as dma-buf. And I can imagine that this is useful outsize virtualization too. I fail to see any common ground for xen-zcopy and udmabuf ... Beside that there is the problem that the udmabuf idea has its own share of issues, for example the fork() issue pointed out by Christian König[1]. So I still need to
Re: [PATCH v2 15/19] omap2: omapfb: allow building it with COMPILE_TEST
On 05/04/18 23:29, Mauro Carvalho Chehab wrote: > This driver builds cleanly with COMPILE_TEST, and it is > needed in order to allow building drivers/media omap2 > driver. > > So, change the logic there to allow building it. > > Signed-off-by: Mauro Carvalho Chehab> --- > drivers/video/fbdev/omap2/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/omap2/Kconfig > b/drivers/video/fbdev/omap2/Kconfig > index 0921c4de8407..82008699d253 100644 > --- a/drivers/video/fbdev/omap2/Kconfig > +++ b/drivers/video/fbdev/omap2/Kconfig > @@ -1,4 +1,4 @@ > -if ARCH_OMAP2PLUS > +if ARCH_OMAP2PLUS || COMPILE_TEST > > source "drivers/video/fbdev/omap2/omapfb/Kconfig" > > Acked-by: Tomi Valkeinen Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [PATCH] media: coda: do not try to propagate format if capture queue busy
On 28/03/18 18:12, Philipp Zabel wrote: The driver helpfully resets the capture queue format and selection rectangle whenever output format is changed. This only works while the capture queue is not busy. Signed-off-by: Philipp Zabel--- drivers/media/platform/coda/coda-common.c | 28 +++- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c index 04e35d70ce2e..d3e22c14fad4 100644 --- a/drivers/media/platform/coda/coda-common.c +++ b/drivers/media/platform/coda/coda-common.c @@ -786,9 +786,8 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv, struct v4l2_format *f) { struct coda_ctx *ctx = fh_to_ctx(priv); - struct coda_q_data *q_data_src; see below struct v4l2_format f_cap; - struct v4l2_rect r; + struct vb2_queue *dst_vq; int ret; ret = coda_try_fmt_vid_out(file, priv, f); @@ -804,23 +803,26 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv, ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc; ctx->quantization = f->fmt.pix.quantization; + dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE); + if (!dst_vq) + return -EINVAL; + + /* +* Setting the capture queue format is not possible while the capture +* queue is still busy. This is not an error, but the user will have to +* make sure themselves that the capture format is set correctly before +* starting the output queue again. +*/ + if (vb2_is_busy(dst_vq)) + return 0; + memset(_cap, 0, sizeof(f_cap)); f_cap.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; coda_g_fmt(file, priv, _cap); f_cap.fmt.pix.width = f->fmt.pix.width; f_cap.fmt.pix.height = f->fmt.pix.height; - ret = coda_try_fmt_vid_cap(file, priv, _cap); - if (ret) - return ret; - - q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT); - r.left = 0; - r.top = 0; - r.width = q_data_src->width; - r.height = q_data_src->height; - - return coda_s_fmt(ctx, _cap, ); + return coda_s_fmt_vid_cap(file, priv, _cap); Is this chunk (and removal of q_data_src above) part of the same patch? It doesn't seem covered by the subject line. Regards, Ian } static int coda_reqbufs(struct file *file, void *priv,
Re: [RfC PATCH] Add udmabuf misc device
On 04/06/2018 03:11 AM, Matt Roper wrote: On Thu, Apr 05, 2018 at 10:32:04PM +0200, Daniel Vetter wrote: Pulling this out of the shadows again. We now also have xen-zcopy from Oleksandr and the hyper dmabuf stuff from Matt and Dongwong. At least from the intel side there seems to be the idea to just have 1 special device that can handle cross-gues/host sharing for all kinds of hypervisors, so I guess you all need to work together :-) Or we throw out the idea that hyper dmabuf will be cross-hypervisor (not sure how useful/reasonable that is, someone please convince me one way or the other). Cheers, Daniel Dongwon (DW) is the one doing all the real work on hyper_dmabuf, but I'm familiar with the use cases he's trying to address, and I think there are a couple high-level goals of his work that are worth calling out as we discuss the various options for sharing buffers produced in one VM with a consumer running in another VM: * We should try to keep the interface/usage separate from the underlying hypervisor implementation details. I.e., in DW's design the sink/source drivers that handle the actual buffer passing in the two VM's should provide a generic interface that does not depend on a specific hypervisor. This is what we did for display, sound and multi-touch on Xen: we have implemented generic protocols which are OS agnostic. Have you started prototyping such a protocol for hyper-dmabuf yet? Behind the scenes there could be various implementations for specific hypervisors (Xen, KVM, ACRN, etc.), and some of those backends may have additional restrictions, but it would be best if userspace didn't have to know the specific hypervisor running on the system and could just query the general capabilities available to it. We've already got projects in flight that are wanting this functionality on Xen and ACRN today. Should we add corresponding communities into discussion then? * The general interface should be able to express sharing from any guest:guest, not just guest:host. Arbitrary G:G sharing might be something some hypervisors simply aren't able to support, but the userspace API itself shouldn't make assumptions or restrict that. I think ideally the sharing API would include some kind of query_targets interface that would return a list of VM's that your current OS is allowed to share with; that list would be depend on the policy established by the system integrator, but obviously wouldn't include targets that the hypervisor itself wouldn't be capable of handling. Can you give a use-case for this? I mean that the system integrator is the one who defines which guests/hosts talk to each other, but querying means that it is possible that VMs have some sort of discovery mechanism, so they can decide on their own whom to connect to. * A lot of the initial use cases are in the realm of graphics, but this shouldn't be a graphics-specific API. Buffers might contain other types of content as well (e.g., audio). Really the content producer could potentially be any driver (or userspace) running in the VM that knows how to import/export dma_buf's (or maybe just import given danvet's suggestion that we should make the sink driver do all the actual memory allocation for any buffers that may be shared). * We need to be able to handle cross-VM coordination of buffer usage as well, so I think we'd want to include fence forwarding support in the API as well to signal back and forth about production/consumption completion. And of course document really well what should happen if, for example, the entire VM you're sharing with/from dies. * The sharing API could be used to share multiple kinds of content in a single system. The sharing sink driver running in the content producer's VM should accept some additional metadata that will be passed over to the target VM as well. The sharing source driver running in the content consumer's VM would then be able to use this metadata to determine the purpose of a new buffer that arrives and filter/dispatch it to the appropriate consumer. For reference, the terminology I'm using is /--\ dma_buf /--\ HV /\ dma_buf /--\ | Producer |--->| Sink | HV | Source |--->| Consumer | \--/ ioctls \--/ HV \/ uevents \--/ In the realm of graphics, "Producer" could potentially be something like an EGL client that sends the buffer at context setup and then signals with fences on each SwapBuffers. "Consumer" could be a Wayland client that proxies the buffers into surfaces or dispatches them to other userspace software that's waiting for buffers. With the hyper_dmabuf approach, there's a lot of ABI details that need to be worked out and really clearly documented before we worry too much about the backend hypervisor-specific
Re: [PATCH v7 2/2] media: video-i2c: add video-i2c driver
Hi Matt, Thanks for the update. A few minor bits, please see below. On Thu, Apr 05, 2018 at 10:14:49PM -0700, Matt Ranostay wrote: > There are several thermal sensors that only have a low-speed bus > interface but output valid video data. This patchset enables support > for the AMG88xx "Grid-Eye" sensor family. > > Signed-off-by: Matt Ranostay> --- > MAINTAINERS | 6 + > drivers/media/i2c/Kconfig | 13 + > drivers/media/i2c/Makefile| 1 + > drivers/media/i2c/video-i2c.c | 560 > ++ > 4 files changed, 580 insertions(+) > create mode 100644 drivers/media/i2c/video-i2c.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index dc153da22e8a..928b6a862626 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14880,6 +14880,12 @@ L: linux-media@vger.kernel.org > S: Maintained > F: drivers/media/platform/video-mux.c > > +VIDEO I2C POLLING DRIVER > +M: Matt Ranostay > +L: linux-media@vger.kernel.org > +S: Maintained > +F: drivers/media/i2c/video-i2c.c > + > VIDEOBUF2 FRAMEWORK > M: Pawel Osciak > M: Marek Szyprowski > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 541f0d28afd8..faaaceb94832 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -974,6 +974,19 @@ config VIDEO_M52790 > >To compile this driver as a module, choose M here: the >module will be called m52790. > + > +config VIDEO_I2C > + tristate "I2C transport video support" > + depends on VIDEO_V4L2 && I2C > + select VIDEOBUF2_VMALLOC > + ---help--- > + Enable the I2C transport video support which supports the > + following: > +* Panasonic AMG88xx Grid-Eye Sensors > + > + To compile this driver as a module, choose M here: the > + module will be called video-i2c > + > endmenu > > menu "Sensors used on soc_camera driver" > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index ea34aee1a85a..84cc472238ef 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -96,6 +96,7 @@ obj-$(CONFIG_VIDEO_LM3646) += lm3646.o > obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o > obj-$(CONFIG_VIDEO_AK881X) += ak881x.o > obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o > +obj-$(CONFIG_VIDEO_I2C) += video-i2c.o > obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o > obj-$(CONFIG_VIDEO_OV2659) += ov2659.o > obj-$(CONFIG_VIDEO_TC358743) += tc358743.o > diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c > new file mode 100644 > index ..42427a724c00 > --- /dev/null > +++ b/drivers/media/i2c/video-i2c.c > @@ -0,0 +1,560 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * video-i2c.c - Support for I2C transport video devices > + * > + * Copyright (C) 2018 Matt Ranostay > + * > + * Supported: > + * - Panasonic AMG88xx Grid-Eye Sensors > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define VIDEO_I2C_DRIVER "video-i2c" > + > +struct video_i2c_chip; > + > +struct video_i2c_buffer { > + struct vb2_v4l2_buffer vb; > + struct list_head list; > +}; > + > +struct video_i2c_data { > + struct i2c_client *client; > + const struct video_i2c_chip *chip; > + struct mutex lock; > + spinlock_t slock; > + unsigned int sequence; > + struct mutex queue_lock; > + > + struct v4l2_device v4l2_dev; > + struct video_device vdev; > + struct vb2_queue vb_vidq; > + > + struct task_struct *kthread_vid_cap; > + struct list_head vid_cap_active; > +}; > + > +static struct v4l2_fmtdesc amg88xx_format = { const? > + .pixelformat = V4L2_PIX_FMT_Y12, > +}; > + > +static struct v4l2_frmsize_discrete amg88xx_size = { const? > + .width = 8, > + .height = 8, > +}; > + > +struct video_i2c_chip { > + /* video dimensions */ > + const struct v4l2_fmtdesc *format; > + const struct v4l2_frmsize_discrete *size; > + > + /* max frames per second */ > + unsigned int max_fps; > + > + /* pixel buffer size */ > + unsigned int buffer_size; > + > + /* pixel size in bits */ > + unsigned int bpp; > + > + /* xfer function */ > + int (*xfer)(struct video_i2c_data *data, char *buf); > +}; > + > +static int amg88xx_xfer(struct video_i2c_data *data, char *buf) > +{ > + struct i2c_client *client = data->client; > + struct i2c_msg msg[2]; > + u8 reg = 0x80; > + int ret; > + > + msg[0].addr = client->addr; > + msg[0].flags = 0; > + msg[0].len = 1; > + msg[0].buf = (char *) > + > + msg[1].addr = client->addr; > + msg[1].flags =