| Message ID | 1536356781-12882-1-git-send-email-satish.nagireddy.nagireddy@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] drm: Add new DIRTY fb flags to pass interlaced alternate fields | expand |
On Fri, Sep 07, 2018 at 02:46:21PM -0700, Satish Kumar Nagireddy wrote: > The requirement is to render interlaced alternate buffers. In case of > alternate, top field and bottom field are in two different buffers. > > The question is, can we pass existing flags DRM_MODE_PRESENT_TOP_FIELD > and DRM_MODE_PRESENT_TOP_FIELD to DRM_IOCTL_MODE_SETPLANE ioctl? The original idea with those flags was bob deinterlacing type of stuff. For fbs with non-interleaved fields I think we'd have to extend addfb somewhat to allow passing a separate buffer for each field. The problem with that is that we only have 4 buffers in addfb, so we'd run out for three plane formats. So we'd have to increase the number of buffers in addfb, or add some kind of implicit assumption on how the fields are stored in the single bo (which I presume might not even be possible on some crazy hardware). > But in case if urrent frame out of display range, application > will invoke DRM_IOCTL_MODE_PAGE_FLIP ioctl. So, it is not guaranteed > that application will always call setplane(), it may also call page_flip(). > Then we will have to introduce two more flags to pass with page_flip() > ioctl, which is complicating things. > > Background of DRM_IOCTL_MODE_DIRTYFB ioctl: This ioctl is defined, so > that userspace can notify the driver that an area of framebuffer has > changed and should be flushed to display hardware. > > The proposed solution is to introduce new dirty fb flags, so that userspace > can pass them with every alternate field buffer and the same is reached > display hardware. This patch adds new dirty framebuffer flags, so that > application can pass interlaced alternate field information to driver. > > Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com> > --- > include/uapi/drm/drm_mode.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index ce7efe2..40fef85 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -428,7 +428,9 @@ struct drm_mode_fb_cmd2 { > > #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01 > #define DRM_MODE_FB_DIRTY_ANNOTATE_FILL 0x02 > -#define DRM_MODE_FB_DIRTY_FLAGS 0x03 > +#define DRM_MODE_FB_DIRTY_TOP_FIELD 0x03 > +#define DRM_MODE_FB_DIRTY_BOTTOM_FIELD 0x04 > +#define DRM_MODE_FB_DIRTY_FLAGS 0x07 > > #define DRM_MODE_FB_DIRTY_MAX_CLIPS 256 > > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Sep 10, 2018 at 03:02:16PM +0300, Ville Syrjälä wrote: > On Fri, Sep 07, 2018 at 02:46:21PM -0700, Satish Kumar Nagireddy wrote: > > The requirement is to render interlaced alternate buffers. In case of > > alternate, top field and bottom field are in two different buffers. > > > > The question is, can we pass existing flags DRM_MODE_PRESENT_TOP_FIELD > > and DRM_MODE_PRESENT_TOP_FIELD to DRM_IOCTL_MODE_SETPLANE ioctl? > > The original idea with those flags was bob deinterlacing type of stuff. > > For fbs with non-interleaved fields I think we'd have to extend addfb > somewhat to allow passing a separate buffer for each field. The problem > with that is that we only have 4 buffers in addfb, so we'd run out for > three plane formats. So we'd have to increase the number of buffers in > addfb, or add some kind of implicit assumption on how the fields are > stored in the single bo (which I presume might not even be possible on > some crazy hardware). Yeah given that an fb is supposed to stick around potentially forever, I think we need to have both fields in one logical framebuffer object. But I have no idea how exactly we'd best go about for this, at least for true interlaced. Doubling up the drm_mode_fb_cmd2 structure (including fourcc and modifiers, or not?) is probably simplest. -Daniel > > But in case if urrent frame out of display range, application > > will invoke DRM_IOCTL_MODE_PAGE_FLIP ioctl. So, it is not guaranteed > > that application will always call setplane(), it may also call page_flip(). > > Then we will have to introduce two more flags to pass with page_flip() > > ioctl, which is complicating things. > > > > Background of DRM_IOCTL_MODE_DIRTYFB ioctl: This ioctl is defined, so > > that userspace can notify the driver that an area of framebuffer has > > changed and should be flushed to display hardware. > > > > The proposed solution is to introduce new dirty fb flags, so that userspace > > can pass them with every alternate field buffer and the same is reached > > display hardware. This patch adds new dirty framebuffer flags, so that > > application can pass interlaced alternate field information to driver. > > > > Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com> > > --- > > include/uapi/drm/drm_mode.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > > index ce7efe2..40fef85 100644 > > --- a/include/uapi/drm/drm_mode.h > > +++ b/include/uapi/drm/drm_mode.h > > @@ -428,7 +428,9 @@ struct drm_mode_fb_cmd2 { > > > > #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01 > > #define DRM_MODE_FB_DIRTY_ANNOTATE_FILL 0x02 > > -#define DRM_MODE_FB_DIRTY_FLAGS 0x03 > > +#define DRM_MODE_FB_DIRTY_TOP_FIELD 0x03 > > +#define DRM_MODE_FB_DIRTY_BOTTOM_FIELD 0x04 > > +#define DRM_MODE_FB_DIRTY_FLAGS 0x07 > > > > #define DRM_MODE_FB_DIRTY_MAX_CLIPS 256 > > > > -- > > 2.7.4 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Ville, Daniel, Thanks a lot for the reply. Some comments below. > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Monday, September 10, 2018 1:28 PM > To: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Satish Kumar Nagireddy <SATISHNA@xilinx.com>; dri- > devel@lists.freedesktop.org > Subject: Re: [RFC PATCH] drm: Add new DIRTY fb flags to pass interlaced > alternate fields > > On Mon, Sep 10, 2018 at 03:02:16PM +0300, Ville Syrjälä wrote: > > On Fri, Sep 07, 2018 at 02:46:21PM -0700, Satish Kumar Nagireddy wrote: > > > The requirement is to render interlaced alternate buffers. In case > > > of alternate, top field and bottom field are in two different buffers. > > > > > > The question is, can we pass existing flags > > > DRM_MODE_PRESENT_TOP_FIELD and DRM_MODE_PRESENT_TOP_FIELD > to DRM_IOCTL_MODE_SETPLANE ioctl? > > > > The original idea with those flags was bob deinterlacing type of stuff. > > > > For fbs with non-interleaved fields I think we'd have to extend addfb > > somewhat to allow passing a separate buffer for each field. The > > problem with that is that we only have 4 buffers in addfb, so we'd run > > out for three plane formats. So we'd have to increase the number of > > buffers in addfb, or add some kind of implicit assumption on how the > > fields are stored in the single bo (which I presume might not even be > > possible on some crazy hardware). > > Yeah given that an fb is supposed to stick around potentially forever, I think > we need to have both fields in one logical framebuffer object. But I have no > idea how exactly we'd best go about for this, at least for true interlaced. > Doubling up the drm_mode_fb_cmd2 structure (including fourcc and > modifiers, or not?) is probably simplest. > -Daniel [satish] As you know In alternate interlaced case each field buffer size is 1920x540 (1920x1080i video resolution) and I have to submit fields one after the other. I don't need to give them together for display. Most of the modern interlaced displays are constructing a frame from a field (by pixel interpolation). Imagine a capture and display use case: Where 1920x540 buffers are captured, top field followed by bottom filed so on... There are chances that some fields can be dropped from capture device. Then userspace will have to communicate DRM with every field buffer if it is top or bottom. Addfb2() will be called only once which cannot be called throughout rendering lifecycle. So I have chosen DIRTY framebuffer ioctl, where some area of fb is changing with every buffer. Regards, Satish > > > > But in case if urrent frame out of display range, application will > > > invoke DRM_IOCTL_MODE_PAGE_FLIP ioctl. So, it is not guaranteed that > > > application will always call setplane(), it may also call page_flip(). > > > Then we will have to introduce two more flags to pass with > > > page_flip() ioctl, which is complicating things. > > > > > > Background of DRM_IOCTL_MODE_DIRTYFB ioctl: This ioctl is defined, > > > so that userspace can notify the driver that an area of framebuffer > > > has changed and should be flushed to display hardware. > > > > > > The proposed solution is to introduce new dirty fb flags, so that > > > userspace can pass them with every alternate field buffer and the > > > same is reached display hardware. This patch adds new dirty > > > framebuffer flags, so that application can pass interlaced alternate field > information to driver. > > > > > > Signed-off-by: Satish Kumar Nagireddy > > > <satish.nagireddy.nagireddy@xilinx.com> > > > --- > > > include/uapi/drm/drm_mode.h | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/uapi/drm/drm_mode.h > > > b/include/uapi/drm/drm_mode.h index ce7efe2..40fef85 100644 > > > --- a/include/uapi/drm/drm_mode.h > > > +++ b/include/uapi/drm/drm_mode.h > > > @@ -428,7 +428,9 @@ struct drm_mode_fb_cmd2 { > > > > > > #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01 #define > > > DRM_MODE_FB_DIRTY_ANNOTATE_FILL 0x02 > > > -#define DRM_MODE_FB_DIRTY_FLAGS 0x03 > > > +#define DRM_MODE_FB_DIRTY_TOP_FIELD 0x03 > > > +#define DRM_MODE_FB_DIRTY_BOTTOM_FIELD 0x04 > > > +#define DRM_MODE_FB_DIRTY_FLAGS 0x07 > > > > > > #define DRM_MODE_FB_DIRTY_MAX_CLIPS 256 > > > > > > -- > > > 2.7.4 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Ville Syrjälä > > Intel > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Tue, Sep 11, 2018 at 02:54:22AM +0000, Satish Kumar Nagireddy wrote: > Hi Ville, Daniel, > > Thanks a lot for the reply. Some comments below. > > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter > > Sent: Monday, September 10, 2018 1:28 PM > > To: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Satish Kumar Nagireddy <SATISHNA@xilinx.com>; dri- > > devel@lists.freedesktop.org > > Subject: Re: [RFC PATCH] drm: Add new DIRTY fb flags to pass interlaced > > alternate fields > > > > On Mon, Sep 10, 2018 at 03:02:16PM +0300, Ville Syrjälä wrote: > > > On Fri, Sep 07, 2018 at 02:46:21PM -0700, Satish Kumar Nagireddy wrote: > > > > The requirement is to render interlaced alternate buffers. In case > > > > of alternate, top field and bottom field are in two different buffers. > > > > > > > > The question is, can we pass existing flags > > > > DRM_MODE_PRESENT_TOP_FIELD and DRM_MODE_PRESENT_TOP_FIELD > > to DRM_IOCTL_MODE_SETPLANE ioctl? > > > > > > The original idea with those flags was bob deinterlacing type of stuff. > > > > > > For fbs with non-interleaved fields I think we'd have to extend addfb > > > somewhat to allow passing a separate buffer for each field. The > > > problem with that is that we only have 4 buffers in addfb, so we'd run > > > out for three plane formats. So we'd have to increase the number of > > > buffers in addfb, or add some kind of implicit assumption on how the > > > fields are stored in the single bo (which I presume might not even be > > > possible on some crazy hardware). > > > > Yeah given that an fb is supposed to stick around potentially forever, I think > > we need to have both fields in one logical framebuffer object. But I have no > > idea how exactly we'd best go about for this, at least for true interlaced. > > Doubling up the drm_mode_fb_cmd2 structure (including fourcc and > > modifiers, or not?) is probably simplest. > > -Daniel > > [satish] As you know In alternate interlaced case each field buffer size > is 1920x540 (1920x1080i video resolution) and I have to submit fields > one after the other. I don't need to give them together for display. > Most of the modern interlaced displays are constructing a frame from a > field (by pixel interpolation). > > Imagine a capture and display use case: Where 1920x540 buffers are > captured, top field followed by bottom filed so on... There are chances > that some fields can be dropped from capture device. Then userspace will > have to communicate DRM with every field buffer if it is top or bottom. > Addfb2() will be called only once which cannot be called > throughout rendering lifecycle. So I have chosen DIRTY framebuffer > ioctl, where some area of fb is changing with every buffer. DRM doesn't work like this, you need to be able to keep showing the same framebuffer forever. That means you need a framebuffer with both fields. So only specifying the field on every flip (dirty fb doesn't do what you want to do here at all) isn't good enough. This is a huge difference in drm compared to v4l, where there's a queue of one-shot buffers. And you can just call addfb for every flip, the overhead is tiny for that. -Daniel
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index ce7efe2..40fef85 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -428,7 +428,9 @@ struct drm_mode_fb_cmd2 { #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01 #define DRM_MODE_FB_DIRTY_ANNOTATE_FILL 0x02 -#define DRM_MODE_FB_DIRTY_FLAGS 0x03 +#define DRM_MODE_FB_DIRTY_TOP_FIELD 0x03 +#define DRM_MODE_FB_DIRTY_BOTTOM_FIELD 0x04 +#define DRM_MODE_FB_DIRTY_FLAGS 0x07 #define DRM_MODE_FB_DIRTY_MAX_CLIPS 256
The requirement is to render interlaced alternate buffers. In case of alternate, top field and bottom field are in two different buffers. The question is, can we pass existing flags DRM_MODE_PRESENT_TOP_FIELD and DRM_MODE_PRESENT_TOP_FIELD to DRM_IOCTL_MODE_SETPLANE ioctl? But in case if urrent frame out of display range, application will invoke DRM_IOCTL_MODE_PAGE_FLIP ioctl. So, it is not guaranteed that application will always call setplane(), it may also call page_flip(). Then we will have to introduce two more flags to pass with page_flip() ioctl, which is complicating things. Background of DRM_IOCTL_MODE_DIRTYFB ioctl: This ioctl is defined, so that userspace can notify the driver that an area of framebuffer has changed and should be flushed to display hardware. The proposed solution is to introduce new dirty fb flags, so that userspace can pass them with every alternate field buffer and the same is reached display hardware. This patch adds new dirty framebuffer flags, so that application can pass interlaced alternate field information to driver. Signed-off-by: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com> --- include/uapi/drm/drm_mode.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)