Interlaced video can have different scan order: Top Field First or Bottom Field First
In case of video with interlaced content, this information should be propagated from the userland to the DRM kernel driver that will process the deinterlacing starting with the top or the bottom field first. That's why we introduce this new flag definition DRM_MODE_FB_BFF (Bottom Field First) that should be used jointly with the already existing DRM_MODE_FB_INTERLACED flag incase of interlaced video with Bottom Field First scan order should be processed.
Fabien Dessenne (2): drm: Add DRM_MODE_FB_BFF flag definition drm/sti: support interlaced top / bottom field first
drivers/gpu/drm/drm_crtc.c | 3 +- drivers/gpu/drm/sti/sti_hqvdp.c | 72 +++++++++++++++++++++++++---------------- include/uapi/drm/drm_mode.h | 1 + 3 files changed, 47 insertions(+), 29 deletions(-)
From: Fabien Dessenne fabien.dessenne@st.com
If a buffer is interlaced, this "Bottom Field First" flag specifies which of the top or the bottom field shall be displayed first. When set, the bottom field shall be displayed first. When unset the top field shall be displayed first.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com --- drivers/gpu/drm/drm_crtc.c | 3 ++- include/uapi/drm/drm_mode.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d40bab2..64b4fdac 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3315,7 +3315,8 @@ internal_framebuffer_create(struct drm_device *dev, struct drm_framebuffer *fb; int ret;
- if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) { + if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS + | DRM_MODE_FB_BFF)) { DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags); return ERR_PTR(-EINVAL); } diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 50adb46..f7c9111 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -354,6 +354,7 @@ struct drm_mode_fb_cmd {
#define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */ #define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */ +#define DRM_MODE_FB_BFF (1<<2) /* if interlaced, bottom field first */
struct drm_mode_fb_cmd2 { __u32 fb_id;
Hi,
Have you any comment for this proposal?
BR Vincent
On 02/12/2016 10:26 AM, Vincent Abriou wrote:
From: Fabien Dessenne fabien.dessenne@st.com
If a buffer is interlaced, this "Bottom Field First" flag specifies which of the top or the bottom field shall be displayed first. When set, the bottom field shall be displayed first. When unset the top field shall be displayed first.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com
drivers/gpu/drm/drm_crtc.c | 3 ++- include/uapi/drm/drm_mode.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d40bab2..64b4fdac 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3315,7 +3315,8 @@ internal_framebuffer_create(struct drm_device *dev, struct drm_framebuffer *fb; int ret;
- if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
- if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS
| DRM_MODE_FB_BFF)) {diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 50adb46..f7c9111 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -354,6 +354,7 @@ struct drm_mode_fb_cmd {
#define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */ #define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */ +#define DRM_MODE_FB_BFF (1<<2) /* if interlaced, bottom field first */
struct drm_mode_fb_cmd2 { __u32 fb_id;
On Fri, Feb 26, 2016 at 11:33:08AM +0100, Vincent ABRIOU wrote:
Hi,
Have you any comment for this proposal?
I guess since we don't really have userspace that uses interlaced modes, much less actually bothers to get the fields correct I think just have some (open-source) userspace somewhere (does gstreamer care enough about this?) which needs this and it's good.
Without userspace this is a hard sell.
Thanks, Daniel
BR Vincent
On 02/12/2016 10:26 AM, Vincent Abriou wrote:
From: Fabien Dessenne fabien.dessenne@st.com
If a buffer is interlaced, this "Bottom Field First" flag specifies which of the top or the bottom field shall be displayed first. When set, the bottom field shall be displayed first. When unset the top field shall be displayed first.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com
drivers/gpu/drm/drm_crtc.c | 3 ++- include/uapi/drm/drm_mode.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d40bab2..64b4fdac 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3315,7 +3315,8 @@ internal_framebuffer_create(struct drm_device *dev, struct drm_framebuffer *fb; int ret;
- if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
- if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS
| DRM_MODE_FB_BFF)) {diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 50adb46..f7c9111 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -354,6 +354,7 @@ struct drm_mode_fb_cmd {
#define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */ #define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */ +#define DRM_MODE_FB_BFF (1<<2) /* if interlaced, bottom field first */
struct drm_mode_fb_cmd2 { __u32 fb_id;
On 02/29/2016 04:32 PM, Daniel Vetter wrote:
On Fri, Feb 26, 2016 at 11:33:08AM +0100, Vincent ABRIOU wrote:
Hi,
Have you any comment for this proposal?
I guess since we don't really have userspace that uses interlaced modes, much less actually bothers to get the fields correct I think just have some (open-source) userspace somewhere (does gstreamer care enough about this?) which needs this and it's good.
Hi Daniel,
At Gstreamer side, an interlaced GstBuffer could be tagged with flags: https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-l...
GST_VIDEO_FRAME_FLAG_TFF is one of them and it is used to warn that the buffer is interlaced with Top Field First (TFF).
At kernel side, we preferred to tag the interlaced buffer with Bottom Field First (BFF) because most of the interlaced buffer are TFF and that BFF are the exception.
Further, we made tests with weston to validate the full path and it is working fine.
BR Vincent
Without userspace this is a hard sell.
Thanks, Daniel
BR Vincent
On 02/12/2016 10:26 AM, Vincent Abriou wrote:
From: Fabien Dessenne fabien.dessenne@st.com
If a buffer is interlaced, this "Bottom Field First" flag specifies which of the top or the bottom field shall be displayed first. When set, the bottom field shall be displayed first. When unset the top field shall be displayed first.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com
drivers/gpu/drm/drm_crtc.c | 3 ++- include/uapi/drm/drm_mode.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d40bab2..64b4fdac 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3315,7 +3315,8 @@ internal_framebuffer_create(struct drm_device *dev, struct drm_framebuffer *fb; int ret;
- if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
- if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS
| DRM_MODE_FB_BFF)) { DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags); return ERR_PTR(-EINVAL);diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 50adb46..f7c9111 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -354,6 +354,7 @@ struct drm_mode_fb_cmd {
#define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */ #define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */ +#define DRM_MODE_FB_BFF (1<<2) /* if interlaced, bottom field first */
struct drm_mode_fb_cmd2 { __u32 fb_id;
On Mon, Feb 29, 2016 at 05:16:13PM +0100, Vincent ABRIOU wrote:
On 02/29/2016 04:32 PM, Daniel Vetter wrote:
On Fri, Feb 26, 2016 at 11:33:08AM +0100, Vincent ABRIOU wrote:
Hi,
Have you any comment for this proposal?
I guess since we don't really have userspace that uses interlaced modes, much less actually bothers to get the fields correct I think just have some (open-source) userspace somewhere (does gstreamer care enough about this?) which needs this and it's good.
Hi Daniel,
At Gstreamer side, an interlaced GstBuffer could be tagged with flags: https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-l...
GST_VIDEO_FRAME_FLAG_TFF is one of them and it is used to warn that the buffer is interlaced with Top Field First (TFF).
At kernel side, we preferred to tag the interlaced buffer with Bottom Field First (BFF) because most of the interlaced buffer are TFF and that BFF are the exception.
Further, we made tests with weston to validate the full path and it is working fine.
Cool, so it's all there. Please add link to the patches/support for this (e.g. Weston patches need to be on the m-l, reviewed by weston maintainers to be considered "ready"). Then once the userspace is all ready we can pull in the kernel patch. After that userspace side can land.
Yes this is a bit of work to orchestrated, but this process is the result of getting it wrong a few too many times in DRM-land. ABI is hard ;-)
So please resend your patch once all that is done, with links to gstreamer and weston patches with their r-b tags from maintainers.
Thanks, Daniel
BR Vincent
Without userspace this is a hard sell.
Thanks, Daniel
BR Vincent
On 02/12/2016 10:26 AM, Vincent Abriou wrote:
From: Fabien Dessenne fabien.dessenne@st.com
If a buffer is interlaced, this "Bottom Field First" flag specifies which of the top or the bottom field shall be displayed first. When set, the bottom field shall be displayed first. When unset the top field shall be displayed first.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com
drivers/gpu/drm/drm_crtc.c | 3 ++- include/uapi/drm/drm_mode.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d40bab2..64b4fdac 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3315,7 +3315,8 @@ internal_framebuffer_create(struct drm_device *dev, struct drm_framebuffer *fb; int ret;
- if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
- if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS
| DRM_MODE_FB_BFF)) { DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags); return ERR_PTR(-EINVAL);diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 50adb46..f7c9111 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -354,6 +354,7 @@ struct drm_mode_fb_cmd {
#define DRM_MODE_FB_INTERLACED (1<<0) /* for interlaced framebuffers */ #define DRM_MODE_FB_MODIFIERS (1<<1) /* enables ->modifer[] */ +#define DRM_MODE_FB_BFF (1<<2) /* if interlaced, bottom field first */
struct drm_mode_fb_cmd2 { __u32 fb_id;
From: Fabien Dessenne fabien.dessenne@st.com
Support top field first and bottom field first interlaced buffers
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com --- drivers/gpu/drm/sti/sti_hqvdp.c | 72 +++++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index e9c33fb..e024c13 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -325,7 +325,7 @@ struct sti_hqvdp_cmd { * @clk_pix_main: pix main clock * @reset: reset control * @vtg_nb: notifier to handle VTG Vsync - * @btm_field_pending: is there any bottom field (interlaced frame) to display + * @nxt_field_pending: is there any other field (interlaced frame) to display * @hqvdp_cmd: buffer of commands * @hqvdp_cmd_paddr: physical address of hqvdp_cmd * @vtg: vtg for main data path @@ -340,7 +340,7 @@ struct sti_hqvdp { struct clk *clk_pix_main; struct reset_control *reset; struct notifier_block vtg_nb; - bool btm_field_pending; + bool nxt_field_pending; void *hqvdp_cmd; dma_addr_t hqvdp_cmd_paddr; struct sti_vtg *vtg; @@ -784,7 +784,7 @@ static void sti_hqvdp_disable(struct sti_hqvdp *hqvdp) * @evt: event message * @data: private data * - * Handle VTG Vsync event, display pending bottom field + * Handle VTG Vsync event, display next field * * RETURNS: * 0 on success. @@ -792,8 +792,8 @@ static void sti_hqvdp_disable(struct sti_hqvdp *hqvdp) int sti_hqvdp_vtg_cb(struct notifier_block *nb, unsigned long evt, void *data) { struct sti_hqvdp *hqvdp = container_of(nb, struct sti_hqvdp, vtg_nb); - int btm_cmd_offset, top_cmd_offest; - struct sti_hqvdp_cmd *btm_cmd, *top_cmd; + int next_cmd_offset, curr_cmd_offest; + struct sti_hqvdp_cmd *next_cmd, *curr_cmd;
if ((evt != VTG_TOP_FIELD_EVENT) && (evt != VTG_BOTTOM_FIELD_EVENT)) { DRM_DEBUG_DRIVER("Unknown event\n"); @@ -808,31 +808,41 @@ int sti_hqvdp_vtg_cb(struct notifier_block *nb, unsigned long evt, void *data) sti_hqvdp_disable(hqvdp); }
- if (hqvdp->btm_field_pending) { - /* Create the btm field command from the current one */ - btm_cmd_offset = sti_hqvdp_get_free_cmd(hqvdp); - top_cmd_offest = sti_hqvdp_get_curr_cmd(hqvdp); - if ((btm_cmd_offset == -1) || (top_cmd_offest == -1)) { + if (hqvdp->nxt_field_pending) { + /* Create the next field command from the current one */ + next_cmd_offset = sti_hqvdp_get_free_cmd(hqvdp); + curr_cmd_offest = sti_hqvdp_get_curr_cmd(hqvdp); + if ((next_cmd_offset == -1) || (curr_cmd_offest == -1)) { DRM_DEBUG_DRIVER("Warning: no cmd, will skip field\n"); return -EBUSY; }
- btm_cmd = hqvdp->hqvdp_cmd + btm_cmd_offset; - top_cmd = hqvdp->hqvdp_cmd + top_cmd_offest; - - memcpy(btm_cmd, top_cmd, sizeof(*btm_cmd)); - - btm_cmd->top.config = TOP_CONFIG_INTER_BTM; - btm_cmd->top.current_luma += - btm_cmd->top.luma_src_pitch / 2; - btm_cmd->top.current_chroma += - btm_cmd->top.chroma_src_pitch / 2; + next_cmd = hqvdp->hqvdp_cmd + next_cmd_offset; + curr_cmd = hqvdp->hqvdp_cmd + curr_cmd_offest; + + memcpy(next_cmd, curr_cmd, sizeof(*next_cmd)); + + if (curr_cmd->top.config == TOP_CONFIG_INTER_TOP) { + /* Display the bottom field now */ + next_cmd->top.config = TOP_CONFIG_INTER_BTM; + next_cmd->top.current_luma += + next_cmd->top.luma_src_pitch / 2; + next_cmd->top.current_chroma += + next_cmd->top.chroma_src_pitch / 2; + } else { + /* Display the top field now */ + next_cmd->top.config = TOP_CONFIG_INTER_TOP; + next_cmd->top.current_luma -= + next_cmd->top.luma_src_pitch / 2; + next_cmd->top.current_chroma -= + next_cmd->top.chroma_src_pitch / 2; + }
/* Post the command to mailbox */ - writel(hqvdp->hqvdp_cmd_paddr + btm_cmd_offset, - hqvdp->regs + HQVDP_MBX_NEXT_CMD); + writel(hqvdp->hqvdp_cmd_paddr + next_cmd_offset, + hqvdp->regs + HQVDP_MBX_NEXT_CMD);
- hqvdp->btm_field_pending = false; + hqvdp->nxt_field_pending = false;
dev_dbg(hqvdp->dev, "%s Posted command:0x%x\n", __func__, hqvdp->hqvdp_cmd_paddr); @@ -1077,7 +1087,7 @@ static int sti_hqvdp_atomic_check(struct drm_plane *drm_plane, return -EINVAL; }
- /* Register VTG Vsync callback to handle bottom fields */ + /* Register VTG Vsync callback to handle bottom/top fields */ if (sti_vtg_register_client(hqvdp->vtg, &hqvdp->vtg_nb, crtc)) { @@ -1175,8 +1185,14 @@ static void sti_hqvdp_atomic_update(struct drm_plane *drm_plane,
/* Handle interlaced */ if (fb->flags & DRM_MODE_FB_INTERLACED) { - /* Top field to display */ - cmd->top.config = TOP_CONFIG_INTER_TOP; + /* Top or bottom field */ + if (fb->flags & DRM_MODE_FB_BFF) { + cmd->top.config = TOP_CONFIG_INTER_BTM; + cmd->top.current_luma += cmd->top.luma_src_pitch; + cmd->top.current_chroma += cmd->top.chroma_src_pitch; + } else { + cmd->top.config = TOP_CONFIG_INTER_TOP; + }
/* Update pitches and vert size */ cmd->top.input_frame_size = (src_h / 2) << 16 | src_w; @@ -1201,9 +1217,9 @@ static void sti_hqvdp_atomic_update(struct drm_plane *drm_plane, writel(hqvdp->hqvdp_cmd_paddr + cmd_offset, hqvdp->regs + HQVDP_MBX_NEXT_CMD);
- /* Interlaced : get ready to display the bottom field at next Vsync */ + /* Interlaced : get ready to display the next field at next Vsync */ if (fb->flags & DRM_MODE_FB_INTERLACED) - hqvdp->btm_field_pending = true; + hqvdp->nxt_field_pending = true;
dev_dbg(hqvdp->dev, "%s Posted command:0x%x\n", __func__, hqvdp->hqvdp_cmd_paddr + cmd_offset);
On Fri, Feb 12, 2016 at 10:26:03AM +0100, Vincent Abriou wrote:
Interlaced video can have different scan order: Top Field First or Bottom Field First
In case of video with interlaced content, this information should be propagated from the userland to the DRM kernel driver that will process the deinterlacing starting with the top or the bottom field first. That's why we introduce this new flag definition DRM_MODE_FB_BFF (Bottom Field First) that should be used jointly with the already existing DRM_MODE_FB_INTERLACED flag incase of interlaced video with Bottom Field First scan order should be processed.
The way I envisioned this long ago is that we would specify the bff/tff at flip time. In fact we already have the DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD flags for setplane. When doing bob deinterlacing these would choose the field we're going to present, and when doing interlaced scanout these would choose tff vs. bff. But that approach does fall short with atomic when you want to flip multiple planes at once.
One problem I see with making this part of the FB is that if you already missed your original deadline for the first field, and you want to actually present the second field instead, you're forced to create another fb. So a plane property might be a bit more flexible. And the same way as the setplane flags we could then share the properties for bob deinterlacing field selection as well. There's no way to do bob deinterlacing with fb flags, unless you create a separate fb for each field.
Hi Ville,
Using DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD assumes that the userland controls the field presentation on the display output. This assumes that the userland is aware of the field actually on display and I think that this information is not provided by the DRM framework. Moreover the field management is something very close to the HW and I do not think that this shall be managed at userland level.
As an alternative approach, the field management can be transparent to the userland, letting the driver doing the job. This is how the STI driver works: when handling an interlaced buffer it displays top and bottom fields at the right time, synchronized with the VSYNC signal (vsync for top field / vsync for btm field). The usage of the atomic mode is transparent for this processing.
Clearly, the two methods are very different. The proposed patch fits with the second one.
BR Fabien
On 02/29/2016 09:41 PM, Ville Syrjälä wrote:
On Fri, Feb 12, 2016 at 10:26:03AM +0100, Vincent Abriou wrote:
Interlaced video can have different scan order: Top Field First or Bottom Field First
In case of video with interlaced content, this information should be propagated from the userland to the DRM kernel driver that will process the deinterlacing starting with the top or the bottom field first. That's why we introduce this new flag definition DRM_MODE_FB_BFF (Bottom Field First) that should be used jointly with the already existing DRM_MODE_FB_INTERLACED flag incase of interlaced video with Bottom Field First scan order should be processed.
The way I envisioned this long ago is that we would specify the bff/tff at flip time. In fact we already have the DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD flags for setplane. When doing bob deinterlacing these would choose the field we're going to present, and when doing interlaced scanout these would choose tff vs. bff. But that approach does fall short with atomic when you want to flip multiple planes at once.
One problem I see with making this part of the FB is that if you already missed your original deadline for the first field, and you want to actually present the second field instead, you're forced to create another fb. So a plane property might be a bit more flexible. And the same way as the setplane flags we could then share the properties for bob deinterlacing field selection as well. There's no way to do bob deinterlacing with fb flags, unless you create a separate fb for each field.
On Thu, Mar 03, 2016 at 11:03:51AM +0100, Fabien DESSENNE wrote:
Hi Ville,
Using DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD assumes that the userland controls the field presentation on the display output. This assumes that the userland is aware of the field actually on display and I think that this information is not provided by the DRM framework. Moreover the field management is something very close to the HW and I do not think that this shall be managed at userland level.
Userland is the only one that knows how the data is to be presented, so I don't understand your comment. Userland is the one that would set your fb flag too.
As an alternative approach, the field management can be transparent to the userland, letting the driver doing the job. This is how the STI driver works: when handling an interlaced buffer it displays top and bottom fields at the right time, synchronized with the VSYNC signal (vsync for top field / vsync for btm field). The usage of the atomic mode is transparent for this processing.
We can do that on most hardware without specific hardware assist. Well, to be precise we can present the first field correctly, but the second/third field will only be presented correctly if the output refresh rate matches the intended refresh rate for the input data. But any hardware assist would have the same problem as well.
Clearly, the two methods are very different.
Not from where I'm sitting.
The proposed patch fits with the second one.
BR Fabien
On 02/29/2016 09:41 PM, Ville Syrjälä wrote:
On Fri, Feb 12, 2016 at 10:26:03AM +0100, Vincent Abriou wrote:
Interlaced video can have different scan order: Top Field First or Bottom Field First
In case of video with interlaced content, this information should be propagated from the userland to the DRM kernel driver that will process the deinterlacing starting with the top or the bottom field first. That's why we introduce this new flag definition DRM_MODE_FB_BFF (Bottom Field First) that should be used jointly with the already existing DRM_MODE_FB_INTERLACED flag incase of interlaced video with Bottom Field First scan order should be processed.
The way I envisioned this long ago is that we would specify the bff/tff at flip time. In fact we already have the DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD flags for setplane. When doing bob deinterlacing these would choose the field we're going to present, and when doing interlaced scanout these would choose tff vs. bff. But that approach does fall short with atomic when you want to flip multiple planes at once.
One problem I see with making this part of the FB is that if you already missed your original deadline for the first field, and you want to actually present the second field instead, you're forced to create another fb. So a plane property might be a bit more flexible. And the same way as the setplane flags we could then share the properties for bob deinterlacing field selection as well. There's no way to do bob deinterlacing with fb flags, unless you create a separate fb for each field.
On 03/03/2016 12:28 PM, Ville Syrjälä wrote:
On Thu, Mar 03, 2016 at 11:03:51AM +0100, Fabien DESSENNE wrote:
Hi Ville,
Using DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD assumes that the userland controls the field presentation on the display output. This assumes that the userland is aware of the field actually on display and I think that this information is not provided by the DRM framework. Moreover the field management is something very close to the HW and I do not think that this shall be managed at userland level.
Userland is the only one that knows how the data is to be presented, so I don't understand your comment. Userland is the one that would set your fb flag too.
Sure, the flag is under userland control. In the two options I am comparing, this flag is set. But in different ways: A/ either the userland sets DRM_MODE_PRESENT_TOP_FIELD to display the top field, then sets DRM_MODE_PRESENT_BOTTOM_FIELD to display the bottom field. (2 PageFlip calls) B/ either, the userland sets DRM_MODE_FB_BFF for a single frame, and lets the driver display the two fields (1 PageFlip call)
As an alternative approach, the field management can be transparent to the userland, letting the driver doing the job. This is how the STI driver works: when handling an interlaced buffer it displays top and bottom fields at the right time, synchronized with the VSYNC signal (vsync for top field / vsync for btm field). The usage of the atomic mode is transparent for this processing.
We can do that on most hardware without specific hardware assist. Well, to be precise we can present the first field correctly, but the second/third field will only be presented correctly if the output refresh rate matches the intended refresh rate for the input data. But any hardware assist would have the same problem as well.
Clearly, the two methods are very different.
Not from where I'm sitting.
The proposed patch fits with the second one.
BR Fabien
On 02/29/2016 09:41 PM, Ville Syrjälä wrote:
On Fri, Feb 12, 2016 at 10:26:03AM +0100, Vincent Abriou wrote:
Interlaced video can have different scan order: Top Field First or Bottom Field First
In case of video with interlaced content, this information should be propagated from the userland to the DRM kernel driver that will process the deinterlacing starting with the top or the bottom field first. That's why we introduce this new flag definition DRM_MODE_FB_BFF (Bottom Field First) that should be used jointly with the already existing DRM_MODE_FB_INTERLACED flag incase of interlaced video with Bottom Field First scan order should be processed.
The way I envisioned this long ago is that we would specify the bff/tff at flip time. In fact we already have the DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD flags for setplane. When doing bob deinterlacing these would choose the field we're going to present, and when doing interlaced scanout these would choose tff vs. bff. But that approach does fall short with atomic when you want to flip multiple planes at once.
One problem I see with making this part of the FB is that if you already missed your original deadline for the first field, and you want to actually present the second field instead, you're forced to create another fb. So a plane property might be a bit more flexible. And the same way as the setplane flags we could then share the properties for bob deinterlacing field selection as well. There's no way to do bob deinterlacing with fb flags, unless you create a separate fb for each field.
On Thu, Mar 03, 2016 at 02:28:38PM +0100, Fabien DESSENNE wrote:
On 03/03/2016 12:28 PM, Ville Syrjälä wrote:
On Thu, Mar 03, 2016 at 11:03:51AM +0100, Fabien DESSENNE wrote:
Hi Ville,
Using DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD assumes that the userland controls the field presentation on the display output. This assumes that the userland is aware of the field actually on display and I think that this information is not provided by the DRM framework. Moreover the field management is something very close to the HW and I do not think that this shall be managed at userland level.
Userland is the only one that knows how the data is to be presented, so I don't understand your comment. Userland is the one that would set your fb flag too.
Sure, the flag is under userland control. In the two options I am comparing, this flag is set. But in different ways: A/ either the userland sets DRM_MODE_PRESENT_TOP_FIELD to display the top field, then sets DRM_MODE_PRESENT_BOTTOM_FIELD to display the bottom field. (2 PageFlip calls)
No, in case of actual interlaced scanout the these flags would in fact mean TFF or BFF. You would only use two pageflips for bob deinterlacing.
B/ either, the userland sets DRM_MODE_FB_BFF for a single frame, and lets the driver display the two fields (1 PageFlip call)
Well, this would rather be N fields, because the scanout being interlaced it will keep alternating between the two fields until another flip is performed.
As an alternative approach, the field management can be transparent to the userland, letting the driver doing the job. This is how the STI driver works: when handling an interlaced buffer it displays top and bottom fields at the right time, synchronized with the VSYNC signal (vsync for top field / vsync for btm field). The usage of the atomic mode is transparent for this processing.
We can do that on most hardware without specific hardware assist. Well, to be precise we can present the first field correctly, but the second/third field will only be presented correctly if the output refresh rate matches the intended refresh rate for the input data. But any hardware assist would have the same problem as well.
Clearly, the two methods are very different.
Not from where I'm sitting.
The proposed patch fits with the second one.
BR Fabien
On 02/29/2016 09:41 PM, Ville Syrjälä wrote:
On Fri, Feb 12, 2016 at 10:26:03AM +0100, Vincent Abriou wrote:
Interlaced video can have different scan order: Top Field First or Bottom Field First
In case of video with interlaced content, this information should be propagated from the userland to the DRM kernel driver that will process the deinterlacing starting with the top or the bottom field first. That's why we introduce this new flag definition DRM_MODE_FB_BFF (Bottom Field First) that should be used jointly with the already existing DRM_MODE_FB_INTERLACED flag incase of interlaced video with Bottom Field First scan order should be processed.
The way I envisioned this long ago is that we would specify the bff/tff at flip time. In fact we already have the DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD flags for setplane. When doing bob deinterlacing these would choose the field we're going to present, and when doing interlaced scanout these would choose tff vs. bff. But that approach does fall short with atomic when you want to flip multiple planes at once.
One problem I see with making this part of the FB is that if you already missed your original deadline for the first field, and you want to actually present the second field instead, you're forced to create another fb. So a plane property might be a bit more flexible. And the same way as the setplane flags we could then share the properties for bob deinterlacing field selection as well. There's no way to do bob deinterlacing with fb flags, unless you create a separate fb for each field.
On 03/03/2016 02:33 PM, Ville Syrjälä wrote:
On Thu, Mar 03, 2016 at 02:28:38PM +0100, Fabien DESSENNE wrote:
On 03/03/2016 12:28 PM, Ville Syrjälä wrote:
On Thu, Mar 03, 2016 at 11:03:51AM +0100, Fabien DESSENNE wrote:
Hi Ville,
Using DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD assumes that the userland controls the field presentation on the display output. This assumes that the userland is aware of the field actually on display and I think that this information is not provided by the DRM framework. Moreover the field management is something very close to the HW and I do not think that this shall be managed at userland level.
Userland is the only one that knows how the data is to be presented, so I don't understand your comment. Userland is the one that would set your fb flag too.
Sure, the flag is under userland control. In the two options I am comparing, this flag is set. But in different ways: A/ either the userland sets DRM_MODE_PRESENT_TOP_FIELD to display the top field, then sets DRM_MODE_PRESENT_BOTTOM_FIELD to display the bottom field. (2 PageFlip calls)
No, in case of actual interlaced scanout the these flags would in fact mean TFF or BFF.
My natural and initial understanding of PRESENT_TOP_FIELD is "present top field *only*". Since these flags are not verbosely documented (and not used in any driver?), I did not catch that it may also mean "present top field *first*"
So considering this statement, I have a better understanding of what you mean and it looks like there are no real difference between the two compared options. At least for the interlaced scanout case.
You would only use two pageflips for bob deinterlacing.
For the bob deinterlacing case (progressive scanout) it is a bit different. But well, no so different.
So focusing back to the initial difference which is about what the flag marks (SetPlane vs fb): DRM_MODE_FB_BFF is a fb flag. The fb can be used in SetPlane, PageFlip or SetCrtc calls. Since DRM_MODE_PRESENT_TOP_FIELD is a drm_mode_set_plane flag I do not see how we can specify the top/bottom field order in a PageFlip call.
B/ either, the userland sets DRM_MODE_FB_BFF for a single frame, and lets the driver display the two fields (1 PageFlip call)
Well, this would rather be N fields, because the scanout being interlaced it will keep alternating between the two fields until another flip is performed.
As an alternative approach, the field management can be transparent to the userland, letting the driver doing the job. This is how the STI driver works: when handling an interlaced buffer it displays top and bottom fields at the right time, synchronized with the VSYNC signal (vsync for top field / vsync for btm field). The usage of the atomic mode is transparent for this processing.
We can do that on most hardware without specific hardware assist. Well, to be precise we can present the first field correctly, but the second/third field will only be presented correctly if the output refresh rate matches the intended refresh rate for the input data. But any hardware assist would have the same problem as well.
Clearly, the two methods are very different.
Not from where I'm sitting.
The proposed patch fits with the second one.
BR Fabien
On 02/29/2016 09:41 PM, Ville Syrjälä wrote:
On Fri, Feb 12, 2016 at 10:26:03AM +0100, Vincent Abriou wrote:
Interlaced video can have different scan order: Top Field First or Bottom Field First
In case of video with interlaced content, this information should be propagated from the userland to the DRM kernel driver that will process the deinterlacing starting with the top or the bottom field first. That's why we introduce this new flag definition DRM_MODE_FB_BFF (Bottom Field First) that should be used jointly with the already existing DRM_MODE_FB_INTERLACED flag incase of interlaced video with Bottom Field First scan order should be processed.
The way I envisioned this long ago is that we would specify the bff/tff at flip time. In fact we already have the DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD flags for setplane. When doing bob deinterlacing these would choose the field we're going to present, and when doing interlaced scanout these would choose tff vs. bff. But that approach does fall short with atomic when you want to flip multiple planes at once.
One problem I see with making this part of the FB is that if you already missed your original deadline for the first field, and you want to actually present the second field instead, you're forced to create another fb. So a plane property might be a bit more flexible. And the same way as the setplane flags we could then share the properties for bob deinterlacing field selection as well. There's no way to do bob deinterlacing with fb flags, unless you create a separate fb for each field.
On Thu, Mar 03, 2016 at 03:40:56PM +0100, Fabien DESSENNE wrote:
On 03/03/2016 02:33 PM, Ville Syrjälä wrote:
On Thu, Mar 03, 2016 at 02:28:38PM +0100, Fabien DESSENNE wrote:
On 03/03/2016 12:28 PM, Ville Syrjälä wrote:
On Thu, Mar 03, 2016 at 11:03:51AM +0100, Fabien DESSENNE wrote:
Hi Ville,
Using DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD assumes that the userland controls the field presentation on the display output. This assumes that the userland is aware of the field actually on display and I think that this information is not provided by the DRM framework. Moreover the field management is something very close to the HW and I do not think that this shall be managed at userland level.
Userland is the only one that knows how the data is to be presented, so I don't understand your comment. Userland is the one that would set your fb flag too.
Sure, the flag is under userland control. In the two options I am comparing, this flag is set. But in different ways: A/ either the userland sets DRM_MODE_PRESENT_TOP_FIELD to display the top field, then sets DRM_MODE_PRESENT_BOTTOM_FIELD to display the bottom field. (2 PageFlip calls)
No, in case of actual interlaced scanout the these flags would in fact mean TFF or BFF.
My natural and initial understanding of PRESENT_TOP_FIELD is "present top field *only*". Since these flags are not verbosely documented (and not used in any driver?), I did not catch that it may also mean "present top field *first*"
So considering this statement, I have a better understanding of what you mean and it looks like there are no real difference between the two compared options. At least for the interlaced scanout case.
You would only use two pageflips for bob deinterlacing.
For the bob deinterlacing case (progressive scanout) it is a bit different. But well, no so different.
So focusing back to the initial difference which is about what the flag marks (SetPlane vs fb): DRM_MODE_FB_BFF is a fb flag. The fb can be used in SetPlane, PageFlip or SetCrtc calls. Since DRM_MODE_PRESENT_TOP_FIELD is a drm_mode_set_plane flag I do not see how we can specify the top/bottom field order in a PageFlip call.
It's all a bit moot since these are legacy APIs and we shouldn't probably worry about them too much. All that really matters IMO is atomic. For atomic we need either a few fb flags which could work for interlaced scanout allowing you to specify tff/bff/any, but as stated this would then force you to create a second fb if you missed your deadline for the first field and want to present the second field instead. But maybe people don't care about deadlines that much?
For the bob deinterlacing we would definitely need some new plane property. I'm not sure how we'd handle cases where the hardware has some more fancy adaptive deinterlacing mechanism that can use weave/bob/something else as needed. I guess it could work with the same kind of property where the flip to the second field could essentially become a nop in case the hardware chose to use weave/etc. when the first field got presented. The other option is providing both fields with the same flip, but then we'd need to add some kind of target framecount/deadline for presenting the second field.
B/ either, the userland sets DRM_MODE_FB_BFF for a single frame, and lets the driver display the two fields (1 PageFlip call)
Well, this would rather be N fields, because the scanout being interlaced it will keep alternating between the two fields until another flip is performed.
As an alternative approach, the field management can be transparent to the userland, letting the driver doing the job. This is how the STI driver works: when handling an interlaced buffer it displays top and bottom fields at the right time, synchronized with the VSYNC signal (vsync for top field / vsync for btm field). The usage of the atomic mode is transparent for this processing.
We can do that on most hardware without specific hardware assist. Well, to be precise we can present the first field correctly, but the second/third field will only be presented correctly if the output refresh rate matches the intended refresh rate for the input data. But any hardware assist would have the same problem as well.
Clearly, the two methods are very different.
Not from where I'm sitting.
The proposed patch fits with the second one.
BR Fabien
On 02/29/2016 09:41 PM, Ville Syrjälä wrote:
On Fri, Feb 12, 2016 at 10:26:03AM +0100, Vincent Abriou wrote: > Interlaced video can have different scan order: > Top Field First or Bottom Field First > > In case of video with interlaced content, this information should be > propagated from the userland to the DRM kernel driver that will process the > deinterlacing starting with the top or the bottom field first. > That's why we introduce this new flag definition DRM_MODE_FB_BFF (Bottom Field > First) that should be used jointly with the already existing > DRM_MODE_FB_INTERLACED flag incase of interlaced video with Bottom Field First > scan order should be processed. The way I envisioned this long ago is that we would specify the bff/tff at flip time. In fact we already have the DRM_MODE_PRESENT_TOP_FIELD/DRM_MODE_PRESENT_BOTTOM_FIELD flags for setplane. When doing bob deinterlacing these would choose the field we're going to present, and when doing interlaced scanout these would choose tff vs. bff. But that approach does fall short with atomic when you want to flip multiple planes at once.
One problem I see with making this part of the FB is that if you already missed your original deadline for the first field, and you want to actually present the second field instead, you're forced to create another fb. So a plane property might be a bit more flexible. And the same way as the setplane flags we could then share the properties for bob deinterlacing field selection as well. There's no way to do bob deinterlacing with fb flags, unless you create a separate fb for each field.
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Fabien DESSENNE -
Ville Syrjälä -
Vincent Abriou -
Vincent ABRIOU