* [PATCH] v4l: soc-camera: Store negotiated buffer settings @ 2011-03-08 0:49 Sergio Aguirre 2011-03-08 7:19 ` Guennadi Liakhovetski 0 siblings, 1 reply; 7+ messages in thread From: Sergio Aguirre @ 2011-03-08 0:49 UTC (permalink / raw) To: g.liakhovetski; +Cc: linux-media, Sergio Aguirre This fixes the problem in which a host driver sets a personalized sizeimage or bytesperline field, and gets ignored when doing G_FMT. Signed-off-by: Sergio Aguirre <saaguirre@ti.com> --- drivers/media/video/soc_camera.c | 9 ++++----- include/media/soc_camera.h | 2 ++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c index a66811b..59dc71d 100644 --- a/drivers/media/video/soc_camera.c +++ b/drivers/media/video/soc_camera.c @@ -363,6 +363,8 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd, icd->user_width = pix->width; icd->user_height = pix->height; icd->colorspace = pix->colorspace; + icd->bytesperline = pix->bytesperline; + icd->sizeimage = pix->sizeimage; icd->vb_vidq.field = icd->field = pix->field; @@ -608,12 +610,9 @@ static int soc_camera_g_fmt_vid_cap(struct file *file, void *priv, pix->height = icd->user_height; pix->field = icd->vb_vidq.field; pix->pixelformat = icd->current_fmt->host_fmt->fourcc; - pix->bytesperline = soc_mbus_bytes_per_line(pix->width, - icd->current_fmt->host_fmt); + pix->bytesperline = icd->bytesperline; pix->colorspace = icd->colorspace; - if (pix->bytesperline < 0) - return pix->bytesperline; - pix->sizeimage = pix->height * pix->bytesperline; + pix->sizeimage = icd->sizeimage; dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n", icd->current_fmt->host_fmt->fourcc); return 0; diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h index 9386db8..de81370 100644 --- a/include/media/soc_camera.h +++ b/include/media/soc_camera.h @@ -30,6 +30,8 @@ struct soc_camera_device { s32 user_width; s32 user_height; enum v4l2_colorspace colorspace; + __u32 bytesperline; /* for padding, zero if unused */ + __u32 sizeimage; unsigned char iface; /* Host number */ unsigned char devnum; /* Device number per host */ struct soc_camera_sense *sense; /* See comment in struct definition */ -- 1.7.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] v4l: soc-camera: Store negotiated buffer settings 2011-03-08 0:49 [PATCH] v4l: soc-camera: Store negotiated buffer settings Sergio Aguirre @ 2011-03-08 7:19 ` Guennadi Liakhovetski 2011-03-08 13:07 ` Sergio Aguirre 0 siblings, 1 reply; 7+ messages in thread From: Guennadi Liakhovetski @ 2011-03-08 7:19 UTC (permalink / raw) To: Sergio Aguirre; +Cc: linux-media On Mon, 7 Mar 2011, Sergio Aguirre wrote: > This fixes the problem in which a host driver > sets a personalized sizeimage or bytesperline field, > and gets ignored when doing G_FMT. Can you tell what that personalised value is? Is it not covered by soc_mbus_bytes_per_line()? Maybe something like a JPEG format? Thanks Guennadi > > Signed-off-by: Sergio Aguirre <saaguirre@ti.com> > --- > drivers/media/video/soc_camera.c | 9 ++++----- > include/media/soc_camera.h | 2 ++ > 2 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c > index a66811b..59dc71d 100644 > --- a/drivers/media/video/soc_camera.c > +++ b/drivers/media/video/soc_camera.c > @@ -363,6 +363,8 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd, > icd->user_width = pix->width; > icd->user_height = pix->height; > icd->colorspace = pix->colorspace; > + icd->bytesperline = pix->bytesperline; > + icd->sizeimage = pix->sizeimage; > icd->vb_vidq.field = > icd->field = pix->field; > > @@ -608,12 +610,9 @@ static int soc_camera_g_fmt_vid_cap(struct file *file, void *priv, > pix->height = icd->user_height; > pix->field = icd->vb_vidq.field; > pix->pixelformat = icd->current_fmt->host_fmt->fourcc; > - pix->bytesperline = soc_mbus_bytes_per_line(pix->width, > - icd->current_fmt->host_fmt); > + pix->bytesperline = icd->bytesperline; > pix->colorspace = icd->colorspace; > - if (pix->bytesperline < 0) > - return pix->bytesperline; > - pix->sizeimage = pix->height * pix->bytesperline; > + pix->sizeimage = icd->sizeimage; > dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n", > icd->current_fmt->host_fmt->fourcc); > return 0; > diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h > index 9386db8..de81370 100644 > --- a/include/media/soc_camera.h > +++ b/include/media/soc_camera.h > @@ -30,6 +30,8 @@ struct soc_camera_device { > s32 user_width; > s32 user_height; > enum v4l2_colorspace colorspace; > + __u32 bytesperline; /* for padding, zero if unused */ > + __u32 sizeimage; > unsigned char iface; /* Host number */ > unsigned char devnum; /* Device number per host */ > struct soc_camera_sense *sense; /* See comment in struct definition */ > -- > 1.7.1 > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] v4l: soc-camera: Store negotiated buffer settings 2011-03-08 7:19 ` Guennadi Liakhovetski @ 2011-03-08 13:07 ` Sergio Aguirre 2011-03-11 16:36 ` Guennadi Liakhovetski 0 siblings, 1 reply; 7+ messages in thread From: Sergio Aguirre @ 2011-03-08 13:07 UTC (permalink / raw) To: Guennadi Liakhovetski; +Cc: linux-media@vger.kernel.org Hi Guennadi, On 03/08/2011 01:19 AM, Guennadi Liakhovetski wrote: > On Mon, 7 Mar 2011, Sergio Aguirre wrote: > >> This fixes the problem in which a host driver >> sets a personalized sizeimage or bytesperline field, >> and gets ignored when doing G_FMT. > > Can you tell what that personalised value is? Is it not covered by > soc_mbus_bytes_per_line()? Maybe something like a JPEG format? In my case, my omap4_camera driver requires to have a bytesperline which is a multiple of 32, and sometimes (depending on the internal HW blocks used) a page aligned byte offset between lines. For example, I want to use such configuration that, for an NV12 buffer, I require a 4K offset between lines, so the vaues are: pix->bytesperline = PAGE_SIZE; pix->sizeimage = pix->bytesperline * height * 3 / 2; Which I filled in TRY_FMT/S_FMT ioctl calls. So, next time a driver tries a G_FMT, it currently gets recalculated by a prefixed table (which comes from soc_mbus_bytes_per_line), which won't give me what i had set before. And it will also recalculate a size image based on this wrong bytesperline * height, which is also wrong, (lacks the * 3 / 2 for NV12). Regards, Sergio > > Thanks > Guennadi > >> >> Signed-off-by: Sergio Aguirre<saaguirre@ti.com> >> --- >> drivers/media/video/soc_camera.c | 9 ++++----- >> include/media/soc_camera.h | 2 ++ >> 2 files changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c >> index a66811b..59dc71d 100644 >> --- a/drivers/media/video/soc_camera.c >> +++ b/drivers/media/video/soc_camera.c >> @@ -363,6 +363,8 @@ static int soc_camera_set_fmt(struct soc_camera_device *icd, >> icd->user_width = pix->width; >> icd->user_height = pix->height; >> icd->colorspace = pix->colorspace; >> + icd->bytesperline = pix->bytesperline; >> + icd->sizeimage = pix->sizeimage; >> icd->vb_vidq.field = >> icd->field = pix->field; >> >> @@ -608,12 +610,9 @@ static int soc_camera_g_fmt_vid_cap(struct file *file, void *priv, >> pix->height = icd->user_height; >> pix->field = icd->vb_vidq.field; >> pix->pixelformat = icd->current_fmt->host_fmt->fourcc; >> - pix->bytesperline = soc_mbus_bytes_per_line(pix->width, >> - icd->current_fmt->host_fmt); >> + pix->bytesperline = icd->bytesperline; >> pix->colorspace = icd->colorspace; >> - if (pix->bytesperline< 0) >> - return pix->bytesperline; >> - pix->sizeimage = pix->height * pix->bytesperline; >> + pix->sizeimage = icd->sizeimage; >> dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n", >> icd->current_fmt->host_fmt->fourcc); >> return 0; >> diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h >> index 9386db8..de81370 100644 >> --- a/include/media/soc_camera.h >> +++ b/include/media/soc_camera.h >> @@ -30,6 +30,8 @@ struct soc_camera_device { >> s32 user_width; >> s32 user_height; >> enum v4l2_colorspace colorspace; >> + __u32 bytesperline; /* for padding, zero if unused */ >> + __u32 sizeimage; >> unsigned char iface; /* Host number */ >> unsigned char devnum; /* Device number per host */ >> struct soc_camera_sense *sense; /* See comment in struct definition */ >> -- >> 1.7.1 >> > > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] v4l: soc-camera: Store negotiated buffer settings 2011-03-08 13:07 ` Sergio Aguirre @ 2011-03-11 16:36 ` Guennadi Liakhovetski 2011-03-11 16:40 ` Sergio Aguirre 0 siblings, 1 reply; 7+ messages in thread From: Guennadi Liakhovetski @ 2011-03-11 16:36 UTC (permalink / raw) To: Sergio Aguirre; +Cc: linux-media@vger.kernel.org On Tue, 8 Mar 2011, Sergio Aguirre wrote: > Hi Guennadi, > > On 03/08/2011 01:19 AM, Guennadi Liakhovetski wrote: > > On Mon, 7 Mar 2011, Sergio Aguirre wrote: > > > > > This fixes the problem in which a host driver > > > sets a personalized sizeimage or bytesperline field, > > > and gets ignored when doing G_FMT. > > > > Can you tell what that personalised value is? Is it not covered by > > soc_mbus_bytes_per_line()? Maybe something like a JPEG format? > > In my case, my omap4_camera driver requires to have a bytesperline which is a > multiple of 32, and sometimes (depending on the internal HW blocks used) a > page aligned byte offset between lines. > > For example, I want to use such configuration that, for an NV12 buffer, I > require a 4K offset between lines, so the vaues are: > > pix->bytesperline = PAGE_SIZE; > pix->sizeimage = pix->bytesperline * height * 3 / 2; > > Which I filled in TRY_FMT/S_FMT ioctl calls. Ok, I think, I agree with this. Until now we didn't have drivers, that wanted to pad data. Even the pxa270 driver adjusts the frame format (see pxa_camera.c::pxa_camera_try_fmt() and the call to v4l_bound_align_image() there in the YUV422P case) to avoid padding, even though that's a different kind of padding - between planes. So, if line padding - as in your case - is indeed needed, I agree, that the correct way to support that is to implement driver-specific bytesperline and sizeimage calculations and, logically, those values should be used in soc_camera_g_fmt_vid_cap(). I'll just change your patch a bit - I'll use "u32" types instead of "__u32" - this is a kernel internal struct and we don't need user-space exported types. Thanks Guennadi > So, next time a driver tries a G_FMT, it currently gets recalculated by > a prefixed table (which comes from soc_mbus_bytes_per_line), which won't give > me what i had set before. And it will also recalculate a size image based on > this wrong bytesperline * height, which is also wrong, (lacks the * 3 / 2 for > NV12). > > Regards, > Sergio > > > > > Thanks > > Guennadi > > > > > > > > Signed-off-by: Sergio Aguirre<saaguirre@ti.com> > > > --- > > > drivers/media/video/soc_camera.c | 9 ++++----- > > > include/media/soc_camera.h | 2 ++ > > > 2 files changed, 6 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/media/video/soc_camera.c > > > b/drivers/media/video/soc_camera.c > > > index a66811b..59dc71d 100644 > > > --- a/drivers/media/video/soc_camera.c > > > +++ b/drivers/media/video/soc_camera.c > > > @@ -363,6 +363,8 @@ static int soc_camera_set_fmt(struct soc_camera_device > > > *icd, > > > icd->user_width = pix->width; > > > icd->user_height = pix->height; > > > icd->colorspace = pix->colorspace; > > > + icd->bytesperline = pix->bytesperline; > > > + icd->sizeimage = pix->sizeimage; > > > icd->vb_vidq.field = > > > icd->field = pix->field; > > > > > > @@ -608,12 +610,9 @@ static int soc_camera_g_fmt_vid_cap(struct file > > > *file, void *priv, > > > pix->height = icd->user_height; > > > pix->field = icd->vb_vidq.field; > > > pix->pixelformat = icd->current_fmt->host_fmt->fourcc; > > > - pix->bytesperline = soc_mbus_bytes_per_line(pix->width, > > > - icd->current_fmt->host_fmt); > > > + pix->bytesperline = icd->bytesperline; > > > pix->colorspace = icd->colorspace; > > > - if (pix->bytesperline< 0) > > > - return pix->bytesperline; > > > - pix->sizeimage = pix->height * pix->bytesperline; > > > + pix->sizeimage = icd->sizeimage; > > > dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n", > > > icd->current_fmt->host_fmt->fourcc); > > > return 0; > > > diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h > > > index 9386db8..de81370 100644 > > > --- a/include/media/soc_camera.h > > > +++ b/include/media/soc_camera.h > > > @@ -30,6 +30,8 @@ struct soc_camera_device { > > > s32 user_width; > > > s32 user_height; > > > enum v4l2_colorspace colorspace; > > > + __u32 bytesperline; /* for padding, zero if unused */ > > > + __u32 sizeimage; > > > unsigned char iface; /* Host number */ > > > unsigned char devnum; /* Device number per host */ > > > struct soc_camera_sense *sense; /* See comment in struct definition */ > > > -- > > > 1.7.1 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] v4l: soc-camera: Store negotiated buffer settings 2011-03-11 16:36 ` Guennadi Liakhovetski @ 2011-03-11 16:40 ` Sergio Aguirre 2011-03-11 16:48 ` Guennadi Liakhovetski 0 siblings, 1 reply; 7+ messages in thread From: Sergio Aguirre @ 2011-03-11 16:40 UTC (permalink / raw) To: Guennadi Liakhovetski; +Cc: linux-media@vger.kernel.org Hi Guennadi, On 03/11/2011 10:36 AM, Guennadi Liakhovetski wrote: > On Tue, 8 Mar 2011, Sergio Aguirre wrote: > >> Hi Guennadi, >> >> On 03/08/2011 01:19 AM, Guennadi Liakhovetski wrote: >>> On Mon, 7 Mar 2011, Sergio Aguirre wrote: >>> >>>> This fixes the problem in which a host driver >>>> sets a personalized sizeimage or bytesperline field, >>>> and gets ignored when doing G_FMT. >>> >>> Can you tell what that personalised value is? Is it not covered by >>> soc_mbus_bytes_per_line()? Maybe something like a JPEG format? >> >> In my case, my omap4_camera driver requires to have a bytesperline which is a >> multiple of 32, and sometimes (depending on the internal HW blocks used) a >> page aligned byte offset between lines. >> >> For example, I want to use such configuration that, for an NV12 buffer, I >> require a 4K offset between lines, so the vaues are: >> >> pix->bytesperline = PAGE_SIZE; >> pix->sizeimage = pix->bytesperline * height * 3 / 2; >> >> Which I filled in TRY_FMT/S_FMT ioctl calls. > > Ok, I think, I agree with this. Until now we didn't have drivers, that > wanted to pad data. Even the pxa270 driver adjusts the frame format (see > pxa_camera.c::pxa_camera_try_fmt() and the call to v4l_bound_align_image() > there in the YUV422P case) to avoid padding, even though that's a > different kind of padding - between planes. So, if line padding - as in > your case - is indeed needed, I agree, that the correct way to support > that is to implement driver-specific bytesperline and sizeimage > calculations and, logically, those values should be used in > soc_camera_g_fmt_vid_cap(). > > I'll just change your patch a bit - I'll use "u32" types instead of > "__u32" - this is a kernel internal struct and we don't need user-space > exported types. Ok, thanks. You're right about u32 type... my bad. So, I'll change that, rebase to: git://linuxtv.org/media_tree.git staging/for_v2.6.39 And resubmit for review. No problem. Regards, Sergio > > Thanks > Guennadi > >> So, next time a driver tries a G_FMT, it currently gets recalculated by >> a prefixed table (which comes from soc_mbus_bytes_per_line), which won't give >> me what i had set before. And it will also recalculate a size image based on >> this wrong bytesperline * height, which is also wrong, (lacks the * 3 / 2 for >> NV12). >> >> Regards, >> Sergio >> >>> >>> Thanks >>> Guennadi >>> >>>> >>>> Signed-off-by: Sergio Aguirre<saaguirre@ti.com> >>>> --- >>>> drivers/media/video/soc_camera.c | 9 ++++----- >>>> include/media/soc_camera.h | 2 ++ >>>> 2 files changed, 6 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/media/video/soc_camera.c >>>> b/drivers/media/video/soc_camera.c >>>> index a66811b..59dc71d 100644 >>>> --- a/drivers/media/video/soc_camera.c >>>> +++ b/drivers/media/video/soc_camera.c >>>> @@ -363,6 +363,8 @@ static int soc_camera_set_fmt(struct soc_camera_device >>>> *icd, >>>> icd->user_width = pix->width; >>>> icd->user_height = pix->height; >>>> icd->colorspace = pix->colorspace; >>>> + icd->bytesperline = pix->bytesperline; >>>> + icd->sizeimage = pix->sizeimage; >>>> icd->vb_vidq.field = >>>> icd->field = pix->field; >>>> >>>> @@ -608,12 +610,9 @@ static int soc_camera_g_fmt_vid_cap(struct file >>>> *file, void *priv, >>>> pix->height = icd->user_height; >>>> pix->field = icd->vb_vidq.field; >>>> pix->pixelformat = icd->current_fmt->host_fmt->fourcc; >>>> - pix->bytesperline = soc_mbus_bytes_per_line(pix->width, >>>> - icd->current_fmt->host_fmt); >>>> + pix->bytesperline = icd->bytesperline; >>>> pix->colorspace = icd->colorspace; >>>> - if (pix->bytesperline< 0) >>>> - return pix->bytesperline; >>>> - pix->sizeimage = pix->height * pix->bytesperline; >>>> + pix->sizeimage = icd->sizeimage; >>>> dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n", >>>> icd->current_fmt->host_fmt->fourcc); >>>> return 0; >>>> diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h >>>> index 9386db8..de81370 100644 >>>> --- a/include/media/soc_camera.h >>>> +++ b/include/media/soc_camera.h >>>> @@ -30,6 +30,8 @@ struct soc_camera_device { >>>> s32 user_width; >>>> s32 user_height; >>>> enum v4l2_colorspace colorspace; >>>> + __u32 bytesperline; /* for padding, zero if unused */ >>>> + __u32 sizeimage; >>>> unsigned char iface; /* Host number */ >>>> unsigned char devnum; /* Device number per host */ >>>> struct soc_camera_sense *sense; /* See comment in struct definition */ >>>> -- >>>> 1.7.1 > > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] v4l: soc-camera: Store negotiated buffer settings 2011-03-11 16:40 ` Sergio Aguirre @ 2011-03-11 16:48 ` Guennadi Liakhovetski 2011-03-11 16:50 ` Sergio Aguirre 0 siblings, 1 reply; 7+ messages in thread From: Guennadi Liakhovetski @ 2011-03-11 16:48 UTC (permalink / raw) To: Sergio Aguirre; +Cc: linux-media@vger.kernel.org On Fri, 11 Mar 2011, Sergio Aguirre wrote: > Hi Guennadi, > > On 03/11/2011 10:36 AM, Guennadi Liakhovetski wrote: > > On Tue, 8 Mar 2011, Sergio Aguirre wrote: > > > > > Hi Guennadi, > > > > > > On 03/08/2011 01:19 AM, Guennadi Liakhovetski wrote: > > > > On Mon, 7 Mar 2011, Sergio Aguirre wrote: > > > > > > > > > This fixes the problem in which a host driver > > > > > sets a personalized sizeimage or bytesperline field, > > > > > and gets ignored when doing G_FMT. > > > > > > > > Can you tell what that personalised value is? Is it not covered by > > > > soc_mbus_bytes_per_line()? Maybe something like a JPEG format? > > > > > > In my case, my omap4_camera driver requires to have a bytesperline which > > > is a > > > multiple of 32, and sometimes (depending on the internal HW blocks used) a > > > page aligned byte offset between lines. > > > > > > For example, I want to use such configuration that, for an NV12 buffer, I > > > require a 4K offset between lines, so the vaues are: > > > > > > pix->bytesperline = PAGE_SIZE; > > > pix->sizeimage = pix->bytesperline * height * 3 / 2; > > > > > > Which I filled in TRY_FMT/S_FMT ioctl calls. > > > > Ok, I think, I agree with this. Until now we didn't have drivers, that > > wanted to pad data. Even the pxa270 driver adjusts the frame format (see > > pxa_camera.c::pxa_camera_try_fmt() and the call to v4l_bound_align_image() > > there in the YUV422P case) to avoid padding, even though that's a > > different kind of padding - between planes. So, if line padding - as in > > your case - is indeed needed, I agree, that the correct way to support > > that is to implement driver-specific bytesperline and sizeimage > > calculations and, logically, those values should be used in > > soc_camera_g_fmt_vid_cap(). > > > > I'll just change your patch a bit - I'll use "u32" types instead of > > "__u32" - this is a kernel internal struct and we don't need user-space > > exported types. > > Ok, thanks. > > You're right about u32 type... my bad. > > So, I'll change that, rebase to: > > git://linuxtv.org/media_tree.git staging/for_v2.6.39 > > And resubmit for review. No problem. Np, I've already committed (locally, not pushed yet) this patch with only changed field types and a bit shuffled lines to group size-related and format-related assignments together. Thanks Guennadi > > Regards, > Sergio > > > > > Thanks > > Guennadi > > > > > So, next time a driver tries a G_FMT, it currently gets recalculated by > > > a prefixed table (which comes from soc_mbus_bytes_per_line), which won't > > > give > > > me what i had set before. And it will also recalculate a size image based > > > on > > > this wrong bytesperline * height, which is also wrong, (lacks the * 3 / 2 > > > for > > > NV12). > > > > > > Regards, > > > Sergio > > > > > > > > > > > Thanks > > > > Guennadi > > > > > > > > > > > > > > Signed-off-by: Sergio Aguirre<saaguirre@ti.com> > > > > > --- > > > > > drivers/media/video/soc_camera.c | 9 ++++----- > > > > > include/media/soc_camera.h | 2 ++ > > > > > 2 files changed, 6 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/drivers/media/video/soc_camera.c > > > > > b/drivers/media/video/soc_camera.c > > > > > index a66811b..59dc71d 100644 > > > > > --- a/drivers/media/video/soc_camera.c > > > > > +++ b/drivers/media/video/soc_camera.c > > > > > @@ -363,6 +363,8 @@ static int soc_camera_set_fmt(struct > > > > > soc_camera_device > > > > > *icd, > > > > > icd->user_width = pix->width; > > > > > icd->user_height = pix->height; > > > > > icd->colorspace = pix->colorspace; > > > > > + icd->bytesperline = pix->bytesperline; > > > > > + icd->sizeimage = pix->sizeimage; > > > > > icd->vb_vidq.field = > > > > > icd->field = pix->field; > > > > > > > > > > @@ -608,12 +610,9 @@ static int soc_camera_g_fmt_vid_cap(struct file > > > > > *file, void *priv, > > > > > pix->height = icd->user_height; > > > > > pix->field = icd->vb_vidq.field; > > > > > pix->pixelformat = icd->current_fmt->host_fmt->fourcc; > > > > > - pix->bytesperline = soc_mbus_bytes_per_line(pix->width, > > > > > - > > > > > icd->current_fmt->host_fmt); > > > > > + pix->bytesperline = icd->bytesperline; > > > > > pix->colorspace = icd->colorspace; > > > > > - if (pix->bytesperline< 0) > > > > > - return pix->bytesperline; > > > > > - pix->sizeimage = pix->height * pix->bytesperline; > > > > > + pix->sizeimage = icd->sizeimage; > > > > > dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n", > > > > > icd->current_fmt->host_fmt->fourcc); > > > > > return 0; > > > > > diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h > > > > > index 9386db8..de81370 100644 > > > > > --- a/include/media/soc_camera.h > > > > > +++ b/include/media/soc_camera.h > > > > > @@ -30,6 +30,8 @@ struct soc_camera_device { > > > > > s32 user_width; > > > > > s32 user_height; > > > > > enum v4l2_colorspace colorspace; > > > > > + __u32 bytesperline; /* for padding, zero if unused */ > > > > > + __u32 sizeimage; > > > > > unsigned char iface; /* Host number */ > > > > > unsigned char devnum; /* Device number per host */ > > > > > struct soc_camera_sense *sense; /* See comment in struct > > > > > definition */ > > > > > -- > > > > > 1.7.1 > > > > --- > > Guennadi Liakhovetski, Ph.D. > > Freelance Open-Source Software Developer > > http://www.open-technology.de/ > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] v4l: soc-camera: Store negotiated buffer settings 2011-03-11 16:48 ` Guennadi Liakhovetski @ 2011-03-11 16:50 ` Sergio Aguirre 0 siblings, 0 replies; 7+ messages in thread From: Sergio Aguirre @ 2011-03-11 16:50 UTC (permalink / raw) To: Guennadi Liakhovetski; +Cc: linux-media@vger.kernel.org On 03/11/2011 10:48 AM, Guennadi Liakhovetski wrote: > On Fri, 11 Mar 2011, Sergio Aguirre wrote: > >> Hi Guennadi, >> >> On 03/11/2011 10:36 AM, Guennadi Liakhovetski wrote: >>> On Tue, 8 Mar 2011, Sergio Aguirre wrote: >>> >>>> Hi Guennadi, >>>> >>>> On 03/08/2011 01:19 AM, Guennadi Liakhovetski wrote: >>>>> On Mon, 7 Mar 2011, Sergio Aguirre wrote: >>>>> >>>>>> This fixes the problem in which a host driver >>>>>> sets a personalized sizeimage or bytesperline field, >>>>>> and gets ignored when doing G_FMT. >>>>> >>>>> Can you tell what that personalised value is? Is it not covered by >>>>> soc_mbus_bytes_per_line()? Maybe something like a JPEG format? >>>> >>>> In my case, my omap4_camera driver requires to have a bytesperline which >>>> is a >>>> multiple of 32, and sometimes (depending on the internal HW blocks used) a >>>> page aligned byte offset between lines. >>>> >>>> For example, I want to use such configuration that, for an NV12 buffer, I >>>> require a 4K offset between lines, so the vaues are: >>>> >>>> pix->bytesperline = PAGE_SIZE; >>>> pix->sizeimage = pix->bytesperline * height * 3 / 2; >>>> >>>> Which I filled in TRY_FMT/S_FMT ioctl calls. >>> >>> Ok, I think, I agree with this. Until now we didn't have drivers, that >>> wanted to pad data. Even the pxa270 driver adjusts the frame format (see >>> pxa_camera.c::pxa_camera_try_fmt() and the call to v4l_bound_align_image() >>> there in the YUV422P case) to avoid padding, even though that's a >>> different kind of padding - between planes. So, if line padding - as in >>> your case - is indeed needed, I agree, that the correct way to support >>> that is to implement driver-specific bytesperline and sizeimage >>> calculations and, logically, those values should be used in >>> soc_camera_g_fmt_vid_cap(). >>> >>> I'll just change your patch a bit - I'll use "u32" types instead of >>> "__u32" - this is a kernel internal struct and we don't need user-space >>> exported types. >> >> Ok, thanks. >> >> You're right about u32 type... my bad. >> >> So, I'll change that, rebase to: >> >> git://linuxtv.org/media_tree.git staging/for_v2.6.39 >> >> And resubmit for review. No problem. > > Np, I've already committed (locally, not pushed yet) this patch with only > changed field types and a bit shuffled lines to group size-related and > format-related assignments together. Oh ok. That's good! Thanks for doing that. :) Thanks, Sergio > > Thanks > Guennadi > <snip> > > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-03-11 16:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-08 0:49 [PATCH] v4l: soc-camera: Store negotiated buffer settings Sergio Aguirre 2011-03-08 7:19 ` Guennadi Liakhovetski 2011-03-08 13:07 ` Sergio Aguirre 2011-03-11 16:36 ` Guennadi Liakhovetski 2011-03-11 16:40 ` Sergio Aguirre 2011-03-11 16:48 ` Guennadi Liakhovetski 2011-03-11 16:50 ` Sergio Aguirre
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox