On Wed, Jan 10, 2018 at 11:53:21AM +0000, Vishal Sagar wrote: > > Hi Vinod > > Thanks for your feedback. > > > -----Original Message----- > > From: Vinod Koul [mailto:vinod.koul@xxxxxxxxx] > > Sent: Friday, December 22, 2017 3:25 PM > > To: Vishal Sagar <vsagar@xxxxxxxxxx> > > Cc: Vishal Sagar <vishal.sagar@xxxxxxxxxx>; dmaengine@xxxxxxxxxxxxxxx; > > Dinesh Kumar <dineshk@xxxxxxxxxx>; michal.simek@xxxxxxxxxx; Jeff > > Mouroux <jmouroux@xxxxxxxxxx>; Radhey Shyam Pandey > > <radheys@xxxxxxxxxx>; Hyun Kwon <hyunk@xxxxxxxxxx>; Maurice Penners > > <mpenner@xxxxxxxxxx>; John Nichols <jnichol@xxxxxxxxxx> > > Subject: Re: [PATCH 2/2] dma: xilinx: Add driver for Video Framebuffer IP > > > > On Fri, Dec 22, 2017 at 08:20:52AM +0000, Vishal Sagar wrote: > > > > > > > > The subsytem name is dmaengine! > > > > > > [Vishal Sagar] > > > > You don't need these tags when replying inline style > > > > Ok noted. > > > > Apologies. This is my first instance of sending across patches to > > community. > > > I saw the precedence for this in other xilinx driver commit messages so > > kept it as it is. > > > I will make the changes as "dmaengine: xilinx: " in next version. > > > > which precedence? > > > > $ git log --online drivers/dma/xilinx/ |grep " dma:" |wc -l > > 2 > > $ git log --online drivers/dma/xilinx/ |grep " dmaengine: " |wc -l > > 52 > > > > My mistake. It was in our git repo. I will take care in future. > > > > > > +static LIST_HEAD(frmbuf_chan_list); static > > > > > +DEFINE_MUTEX(frmbuf_chan_list_lock); > > > > > > > > why do you need these globals? > > > > Please fix your MUA to wrap at 80 columns, I have reflowed your text to fit. > > Ok thanks. I am using Outlook to send mails and have setup the client as > per https://elinux.org/Mail_client_tips. Did you follow that? Esp the wrapping part. Whatever tool you use, pls make sure replies are within 80chars > > > > > > The frame buffer read and write are Soft IPs which can be configured > > > to support different video formats. Please refer to PG278 Product > > > Guide for Video Frame Buffer Read and Write v2.0. > > > > huh why should I refer to that! > > The product guide gives details about the memory layout of buffer for > every format supported by these IPs. Sorry but not my problem. I should understand and be able to review the patch. Please create and add comments to satisfy that > > > > > > The client driver would want to > > > know what are the video formats supported and set the format > > accordingly. > > > For this there are APIs exposed by this driver. > > > > You are *not* the first driver to support these kind of use cases. > 80 char wrap issue again :( > Ok. We discussed internally about this. Instead of using custom APIs we > could use device_config() in the client and send custom data by wrapping > it around the struct dma_slave_config. > ===================================================================== > enum { > UNKNOWN, > XFRMBUF, > } xdma_type; > > struct xilinx_dma_config { > struct dma_slave_config config; > enum xdma_type type; > union { > struct framebuffer_config; > /* Other future dma configs here */ > } xdma_config; > }; again, you are not the first ones proposing this and sorry we don't go that route anymore. Please see other video drivers on how they set this > > enum { > UNKNOWN, > V4L2, > DRM > } framework_type; > > struct framebuffer_config { > video_format; /* input param */ > enum framework_type fwtype; > supported_v4l2_video_formats[]; /* output param */ > supported_drm_video_formats[]; /* output param */ > } this is all video specfic stuff, why should a dmaengine driver care about this. DMA care about transferring data. As I said last time, you should split video stuff and dmaengine stuff to separate drivers. Dmaengine should only worry about transferring data and video driver should worry about video formats > ===================================================================== > First time client sets type = UNKNOWN and calls device_config(). The > driver sets the type back as XFRMBUF and populates the supported v4l2 and > drm video formats and returns. Client calls it second time with type = > XFRMBUF and video_format and fwtype set to one of the supported formats. > This will help framebuffer driver correctly configure the video format. who is the client, video driver right? -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
