RE: [PATCH 09/38] drm/sti/sti_hqvdp: Fix incorrectly named function 'sti_hqvdp_vtg_cb()'
Hi Lee Thank you for the patch BR Fabien ST Restricted > -Original Message- > From: Lee Jones > Sent: jeudi 20 mai 2021 14:02 > To: lee.jo...@linaro.org > Cc: linux-ker...@vger.kernel.org; Benjamin Gaignard > ; David Airlie ; Daniel Vetter > ; Philipp Zabel ; Fabien DESSENNE > ; dri-devel@lists.freedesktop.org > Subject: [PATCH 09/38] drm/sti/sti_hqvdp: Fix incorrectly named function > 'sti_hqvdp_vtg_cb()' > > Fixes the following W=1 kernel build warning(s): > > drivers/gpu/drm/sti/sti_hqvdp.c:796: warning: expecting prototype for > sti_vdp_vtg_cb(). Prototype was for sti_hqvdp_vtg_cb() instead > > Cc: Benjamin Gaignard > Cc: David Airlie > Cc: Daniel Vetter > Cc: Philipp Zabel > Cc: Fabien Dessenne > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Lee Jones Reviewed-by: Fabien Dessenne > --- > drivers/gpu/drm/sti/sti_hqvdp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c > index edbb99f53de19..d09b08995b12a 100644 > --- a/drivers/gpu/drm/sti/sti_hqvdp.c > +++ b/drivers/gpu/drm/sti/sti_hqvdp.c > @@ -782,7 +782,7 @@ static void sti_hqvdp_disable(struct sti_hqvdp *hqvdp) } > > /** > - * sti_vdp_vtg_cb > + * sti_hqvdp_vtg_cb > * @nb: notifier block > * @evt: event message > * @data: private data > -- > 2.31.1
RE: [PATCH 07/38] drm/sti/sti_hda: Provide missing function names
Hi Lee Thank you for the patch BR Fabien ST Restricted > -Original Message- > From: Lee Jones > Sent: jeudi 20 mai 2021 14:02 > To: lee.jo...@linaro.org > Cc: linux-ker...@vger.kernel.org; Benjamin Gaignard > ; David Airlie ; Daniel Vetter > ; Fabien DESSENNE ; dri- > de...@lists.freedesktop.org > Subject: [PATCH 07/38] drm/sti/sti_hda: Provide missing function names > > Fixes the following W=1 kernel build warning(s): > > drivers/gpu/drm/sti/sti_hda.c:283: warning: expecting prototype for Search > for > a video mode in the supported modes table(). Prototype was for > hda_get_mode_idx() instead > drivers/gpu/drm/sti/sti_hda.c:301: warning: expecting prototype for Enable > the > HD DACS(). Prototype was for hda_enable_hd_dacs() instead > drivers/gpu/drm/sti/sti_hda.c:383: warning: This comment starts with '/**', > but > isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst > > Cc: Benjamin Gaignard > Cc: David Airlie > Cc: Daniel Vetter > Cc: Fabien Dessenne > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Lee Jones Reviewed-by: Fabien Dessenne > --- > drivers/gpu/drm/sti/sti_hda.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c > index > 5c2b650b561d5..03f3377f918c0 100644 > --- a/drivers/gpu/drm/sti/sti_hda.c > +++ b/drivers/gpu/drm/sti/sti_hda.c > @@ -272,7 +272,7 @@ static void hda_write(struct sti_hda *hda, u32 val, int > offset) } > > /** > - * Search for a video mode in the supported modes table > + * hda_get_mode_idx - Search for a video mode in the supported modes > + table > * > * @mode: mode being searched > * @idx: index of the found mode > @@ -292,7 +292,7 @@ static bool hda_get_mode_idx(struct > drm_display_mode mode, int *idx) } > > /** > - * Enable the HD DACS > + * hda_enable_hd_dacs - Enable the HD DACS > * > * @hda: pointer to HD analog structure > * @enable: true if HD DACS need to be enabled, else false @@ -380,7 +380,7 > @@ static void hda_debugfs_init(struct sti_hda *hda, struct drm_minor *minor) > } > > /** > - * Configure AWG, writing instructions > + * sti_hda_configure_awg - Configure AWG, writing instructions > * > * @hda: pointer to HD analog structure > * @awg_instr: pointer to AWG instructions table > -- > 2.31.1
Re: [PATCH 2/4] media: bdisp: fix memleak on release
Hi Johan
Thank you for the patch
BR
Fabien
On 10/10/2019 3:13 PM, Johan Hovold wrote:
> If a process is interrupted while accessing the video device and the
> device lock is contended, release() could return early and fail to free
> related resources.
>
> Note that the return value of the v4l2 release file operation is
> ignored.
>
> Fixes: 28ffeebbb7bd ("[media] bdisp: 2D blitter driver using v4l2 mem2mem
> framework")
> Cc: stable # 4.2
> Cc: Fabien Dessenne
> Cc: Hans Verkuil
> Cc: Mauro Carvalho Chehab
> Signed-off-by: Johan Hovold
Reviewed-by: Fabien Dessenne
> ---
> drivers/media/platform/sti/bdisp/bdisp-v4l2.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
> b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
> index e90f1ba30574..675b5f2b4c2e 100644
> --- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
> +++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
> @@ -651,8 +651,7 @@ static int bdisp_release(struct file *file)
>
> dev_dbg(bdisp->dev, "%s\n", __func__);
>
> - if (mutex_lock_interruptible(>lock))
> - return -ERESTARTSYS;
> + mutex_lock(>lock);
>
> v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
>
[PATCH 0/2] drm/stm: ltdc: manage error cases in probe
This patchset adds some check of the returned error code in probe. Fabien Dessenne (2): drm/stm: ltdc: manage the get_irq probe defer case drm/stm: ltdc: return appropriate error code during probe drivers/gpu/drm/stm/ltdc.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/stm: ltdc: manage the get_irq probe defer case
Manage the -EPROBE_DEFER error case for the ltdc IRQ.
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/stm/ltdc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 566b0d8..521ba83 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -1174,6 +1174,9 @@ int ltdc_load(struct drm_device *ddev)
for (i = 0; i < MAX_IRQ; i++) {
irq = platform_get_irq(pdev, i);
+ if (irq == -EPROBE_DEFER)
+ goto err;
+
if (irq < 0)
continue;
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/stm: ltdc: return appropriate error code during probe
During probe, return the "clk_get" error value instead of -ENODEV.
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/stm/ltdc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 521ba83..97912e2 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -1145,8 +1145,9 @@ int ltdc_load(struct drm_device *ddev)
ldev->pixel_clk = devm_clk_get(dev, "lcd");
if (IS_ERR(ldev->pixel_clk)) {
- DRM_ERROR("Unable to get lcd clock\n");
- return -ENODEV;
+ if (PTR_ERR(ldev->pixel_clk) != -EPROBE_DEFER)
+ DRM_ERROR("Unable to get lcd clock\n");
+ return PTR_ERR(ldev->pixel_clk);
}
if (clk_prepare_enable(ldev->pixel_clk)) {
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/sti: Fix up crtc_state->event handling
Use drm-core to handle event.
This is required to be able to use the nonblocking helpers.
Signed-off-by: Fabien Dessenne <fabien.desse...@st.com>
---
drivers/gpu/drm/sti/sti_crtc.c | 46 +
drivers/gpu/drm/sti/sti_mixer.h | 2 --
2 files changed, 14 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c
index e992bed..d45a433 100644
--- a/drivers/gpu/drm/sti/sti_crtc.c
+++ b/drivers/gpu/drm/sti/sti_crtc.c
@@ -134,21 +134,6 @@ sti_crtc_mode_set_nofb(struct drm_crtc *crtc)
sti_crtc_mode_set(crtc, >state->adjusted_mode);
}
-static void sti_crtc_atomic_begin(struct drm_crtc *crtc,
- struct drm_crtc_state *old_crtc_state)
-{
- struct sti_mixer *mixer = to_sti_mixer(crtc);
-
- if (crtc->state->event) {
- crtc->state->event->pipe = drm_crtc_index(crtc);
-
- WARN_ON(drm_crtc_vblank_get(crtc) != 0);
-
- mixer->pending_event = crtc->state->event;
- crtc->state->event = NULL;
- }
-}
-
static void sti_crtc_atomic_flush(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
{
@@ -156,6 +141,8 @@ static void sti_crtc_atomic_flush(struct drm_crtc *crtc,
struct sti_mixer *mixer = to_sti_mixer(crtc);
struct sti_compositor *compo = dev_get_drvdata(mixer->dev);
struct drm_plane *p;
+ struct drm_pending_vblank_event *event;
+ unsigned long flags;
DRM_DEBUG_DRIVER("\n");
@@ -220,13 +207,24 @@ static void sti_crtc_atomic_flush(struct drm_crtc *crtc,
break;
}
}
+
+ event = crtc->state->event;
+ if (event) {
+ crtc->state->event = NULL;
+
+ spin_lock_irqsave(>dev->event_lock, flags);
+ if (drm_crtc_vblank_get(crtc) == 0)
+ drm_crtc_arm_vblank_event(crtc, event);
+ else
+ drm_crtc_send_vblank_event(crtc, event);
+ spin_unlock_irqrestore(>dev->event_lock, flags);
+ }
}
static const struct drm_crtc_helper_funcs sti_crtc_helper_funcs = {
.enable = sti_crtc_enable,
.disable = sti_crtc_disabling,
.mode_set_nofb = sti_crtc_mode_set_nofb,
- .atomic_begin = sti_crtc_atomic_begin,
.atomic_flush = sti_crtc_atomic_flush,
};
@@ -250,7 +248,6 @@ int sti_crtc_vblank_cb(struct notifier_block *nb,
struct sti_compositor *compo;
struct drm_crtc *crtc = data;
struct sti_mixer *mixer;
- unsigned long flags;
struct sti_private *priv;
unsigned int pipe;
@@ -267,14 +264,6 @@ int sti_crtc_vblank_cb(struct notifier_block *nb,
drm_crtc_handle_vblank(crtc);
- spin_lock_irqsave(>dev->event_lock, flags);
- if (mixer->pending_event) {
- drm_crtc_send_vblank_event(crtc, mixer->pending_event);
- drm_crtc_vblank_put(crtc);
- mixer->pending_event = NULL;
- }
- spin_unlock_irqrestore(>dev->event_lock, flags);
-
if (mixer->status == STI_MIXER_DISABLING) {
struct drm_plane *p;
@@ -317,19 +306,12 @@ void sti_crtc_disable_vblank(struct drm_device *drm_dev,
unsigned int pipe)
struct sti_private *priv = drm_dev->dev_private;
struct sti_compositor *compo = priv->compo;
struct notifier_block *vtg_vblank_nb = >vtg_vblank_nb[pipe];
- struct drm_crtc *crtc = >mixer[pipe]->drm_crtc;
struct sti_vtg *vtg = compo->vtg[pipe];
DRM_DEBUG_DRIVER("\n");
if (sti_vtg_unregister_client(vtg, vtg_vblank_nb))
DRM_DEBUG_DRIVER("Warning: cannot unregister VTG notifier\n");
-
- /* free the resources of the pending requests */
- if (compo->mixer[pipe]->pending_event) {
- drm_crtc_vblank_put(crtc);
- compo->mixer[pipe]->pending_event = NULL;
- }
}
static int sti_crtc_late_register(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/sti/sti_mixer.h b/drivers/gpu/drm/sti/sti_mixer.h
index 830a3c4..e64a00e 100644
--- a/drivers/gpu/drm/sti/sti_mixer.h
+++ b/drivers/gpu/drm/sti/sti_mixer.h
@@ -28,7 +28,6 @@ enum sti_mixer_status {
* @regs: mixer registers
* @id: id of the mixer
* @drm_crtc: crtc object link to the mixer
- * @pending_event: set if a flip event is pending on crtc
* @status: to know the status of the mixer
*/
struct sti_mixer {
@@ -36,7 +35,6 @@ struct sti_mixer {
void __iomem *regs;
int id;
struct drm_crtc drm_crtc;
- struct drm_pending_vblank_event *pending_event;
enum sti_mixer_status status;
};
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/sti: do not check hw scaling if mode is not set
Fix a division by 0 case : in some cases, when the HQVDP plane is being
disabled atomic_check() is called with "mode->clock = 0".
In that case, do not check for scaling capabilities.
Signed-off-by: Fabien Dessenne <fabien.desse...@st.com>
---
drivers/gpu/drm/sti/sti_hqvdp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index f88130f..723ac30 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1035,9 +1035,9 @@ static int sti_hqvdp_atomic_check(struct drm_plane
*drm_plane,
src_w = state->src_w >> 16;
src_h = state->src_h >> 16;
- if (!sti_hqvdp_check_hw_scaling(hqvdp, mode,
- src_w, src_h,
- dst_w, dst_h)) {
+ if (mode->clock && !sti_hqvdp_check_hw_scaling(hqvdp, mode,
+ src_w, src_h,
+ dst_w, dst_h)) {
DRM_ERROR("Scaling beyond HW capabilities\n");
return -EINVAL;
}
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/sti: use atomic_helper for commit
Since nonblocking atomic commits are now supported, the driver can
now use drm_atomic_helper_commit().
Signed-off-by: Fabien Dessenne <fabien.desse...@st.com>
---
drivers/gpu/drm/sti/sti_drv.c | 83 +--
drivers/gpu/drm/sti/sti_drv.h | 6
2 files changed, 1 insertion(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index ff71e25..9a9b9a2 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -150,52 +150,6 @@ static void sti_drm_dbg_cleanup(struct drm_minor *minor)
1, minor);
}
-static void sti_atomic_schedule(struct sti_private *private,
- struct drm_atomic_state *state)
-{
- private->commit.state = state;
- schedule_work(>commit.work);
-}
-
-static void sti_atomic_complete(struct sti_private *private,
- struct drm_atomic_state *state)
-{
- struct drm_device *drm = private->drm_dev;
-
- /*
-* Everything below can be run asynchronously without the need to grab
-* any modeset locks at all under one condition: It must be guaranteed
-* that the asynchronous work has either been cancelled (if the driver
-* supports it, which at least requires that the framebuffers get
-* cleaned up with drm_atomic_helper_cleanup_planes()) or completed
-* before the new state gets committed on the software side with
-* drm_atomic_helper_swap_state().
-*
-* This scheme allows new atomic state updates to be prepared and
-* checked in parallel to the asynchronous completion of the previous
-* update. Which is important since compositors need to figure out the
-* composition of the next frame right after having submitted the
-* current layout.
-*/
-
- drm_atomic_helper_commit_modeset_disables(drm, state);
- drm_atomic_helper_commit_planes(drm, state, 0);
- drm_atomic_helper_commit_modeset_enables(drm, state);
-
- drm_atomic_helper_wait_for_vblanks(drm, state);
-
- drm_atomic_helper_cleanup_planes(drm, state);
- drm_atomic_state_put(state);
-}
-
-static void sti_atomic_work(struct work_struct *work)
-{
- struct sti_private *private = container_of(work,
- struct sti_private, commit.work);
-
- sti_atomic_complete(private, private->commit.state);
-}
-
static int sti_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
@@ -216,38 +170,6 @@ static int sti_atomic_check(struct drm_device *dev,
return ret;
}
-static int sti_atomic_commit(struct drm_device *drm,
-struct drm_atomic_state *state, bool nonblock)
-{
- struct sti_private *private = drm->dev_private;
- int err;
-
- err = drm_atomic_helper_prepare_planes(drm, state);
- if (err)
- return err;
-
- /* serialize outstanding nonblocking commits */
- mutex_lock(>commit.lock);
- flush_work(>commit.work);
-
- /*
-* This is the point of no return - everything below never fails except
-* when the hw goes bonghits. Which means we can commit the new state on
-* the software side now.
-*/
-
- drm_atomic_helper_swap_state(state, true);
-
- drm_atomic_state_get(state);
- if (nonblock)
- sti_atomic_schedule(private, state);
- else
- sti_atomic_complete(private, state);
-
- mutex_unlock(>commit.lock);
- return 0;
-}
-
static void sti_output_poll_changed(struct drm_device *ddev)
{
struct sti_private *private = ddev->dev_private;
@@ -271,7 +193,7 @@ static const struct drm_mode_config_funcs
sti_mode_config_funcs = {
.fb_create = drm_fb_cma_create,
.output_poll_changed = sti_output_poll_changed,
.atomic_check = sti_atomic_check,
- .atomic_commit = sti_atomic_commit,
+ .atomic_commit = drm_atomic_helper_commit,
};
static void sti_mode_config_init(struct drm_device *dev)
@@ -352,9 +274,6 @@ static int sti_init(struct drm_device *ddev)
dev_set_drvdata(ddev->dev, ddev);
private->drm_dev = ddev;
- mutex_init(>commit.lock);
- INIT_WORK(>commit.work, sti_atomic_work);
-
drm_mode_config_init(ddev);
sti_mode_config_init(ddev);
diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h
index 78ebe5e..f9276cd 100644
--- a/drivers/gpu/drm/sti/sti_drv.h
+++ b/drivers/gpu/drm/sti/sti_drv.h
@@ -25,12 +25,6 @@ struct sti_private {
struct drm_property *plane_zorder_property;
struct drm_device *drm_dev;
struct drm_fbdev_cma *fbdev;
-
- struct {
- struct drm_atomic_state *state;
- struct work_struct work;
- struct mutex lock;
- }
[PATCH 0/3] Use nonblocking atomic helpers
This series replaces the driver customized atomic_commit implementation with the drm core helpers (patch 1). The vblank event management is reworked (patch 2) and a side-effect is fixed (patch 3). Fabien ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/5] drm/sti: do not sync legacy IOCTL on vblank if not ATOMIC
Hi Daniel I come to the conclusion that (only) Atomic Weston will solve all of my troubles, so let's forget these patches (and work for atomic weston). By the way, is the expected behavior (Vblank - sync'ed or not) of drmModeSetPlane() described anywhere? The last time I browsed the related documentation I could not find the answer. Maybe something that needs to be clarified ? I will also re-send patch 1 & 5 out of this abandoned series since they make sense independently of this (a)sync stuff. BR. Fabien On 04/01/17 10:18, Daniel Vetter wrote: > On Tue, Jan 03, 2017 at 05:56:47PM +0100, Fabien Dessenne wrote: >> These patches allow a legacy framework (eg non-atomic Weston) to call >> several SETPLANE within the same Vsync cycle. >> - [PATCH 1/5] drm/sti: use atomic_helper for commit >> - [PATCH 2/5] drm/sti: add drm_file to sti_private >> - [PATCH 3/5] drm/sti: do not sync SETPLANE on vblank if not ATOMIC >> - [PATCH 4/5] drm/sti: do not sync SETPROPERTY on vblank if not ATOMIC >> - [PATCH 5/5] drm/sti: do not check hw scaling if mode is not set > Upstream weston never really enabled plane support, why exactly do you > need this? Also, if this really is required, I think we should implement > something like this (aka async plane flips) in general for everyone. sti > is by far not the only driver playing around with these games. > -Daniel
[PATCH 5/5] drm/sti: do not check hw scaling if mode is not set
Fix a division by 0 case : in some cases, when the HQVDP plane is being
disabled atomic_check() is called with "mode->clock = 0".
In that case, do not check for scaling capabilities.
Change-Id: I7fb752ab394211c3deafa149f52cfb2bca244e84
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_hqvdp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index 3fd6f3a..fe36e0f 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1035,9 +1035,9 @@ static int sti_hqvdp_atomic_check(struct drm_plane
*drm_plane,
src_w = state->src_w >> 16;
src_h = state->src_h >> 16;
- if (!sti_hqvdp_check_hw_scaling(hqvdp, mode,
- src_w, src_h,
- dst_w, dst_h)) {
+ if (mode->clock && !sti_hqvdp_check_hw_scaling(hqvdp, mode,
+ src_w, src_h,
+ dst_w, dst_h)) {
DRM_ERROR("Scaling beyond HW capabilities\n");
return -EINVAL;
}
--
2.7.4
[PATCH 4/5] drm/sti: do not sync SETPROPERTY on vblank if not ATOMIC
If the client does not set the ATOMIC capability, do not wait for vblank
before returning an DRM_IOCTL_MODE_OBJ_SETPROPERTY call.
In this way, a legacy framework (eg non-atomic Weston) can call several
SETPROPERTY within the same Vsync cycle.
This is implemented by setting the legacy_cursor_update flag, to behave
the same way as DRM_IOCTL_MODE_CURSOR (not vblank synced).
Change-Id: I6b6134eca57eca399bdda006ab1cb8280d4002d4
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_cursor.c | 2 +-
drivers/gpu/drm/sti/sti_gdp.c| 2 +-
drivers/gpu/drm/sti/sti_hqvdp.c | 2 +-
drivers/gpu/drm/sti/sti_plane.c | 54
drivers/gpu/drm/sti/sti_plane.h | 2 ++
5 files changed, 59 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index ea0dbae..d7e9f8a 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -349,7 +349,7 @@ static const struct drm_plane_funcs
sti_cursor_plane_helpers_funcs = {
.update_plane = sti_plane_update_plane,
.disable_plane = sti_plane_disable_plane,
.destroy = sti_cursor_destroy,
- .set_property = drm_atomic_helper_plane_set_property,
+ .set_property = sti_plane_set_property,
.reset = sti_plane_reset,
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index a379bbe..6fa5042 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -885,7 +885,7 @@ static const struct drm_plane_funcs
sti_gdp_plane_helpers_funcs = {
.update_plane = sti_plane_update_plane,
.disable_plane = sti_plane_disable_plane,
.destroy = sti_gdp_destroy,
- .set_property = drm_atomic_helper_plane_set_property,
+ .set_property = sti_plane_set_property,
.reset = sti_plane_reset,
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index 65ca43f..3fd6f3a 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1255,7 +1255,7 @@ static const struct drm_plane_funcs
sti_hqvdp_plane_helpers_funcs = {
.update_plane = sti_plane_update_plane,
.disable_plane = sti_plane_disable_plane,
.destroy = sti_hqvdp_destroy,
- .set_property = drm_atomic_helper_plane_set_property,
+ .set_property = sti_plane_set_property,
.reset = sti_plane_reset,
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
index 22cf30d..c58fe1b 100644
--- a/drivers/gpu/drm/sti/sti_plane.c
+++ b/drivers/gpu/drm/sti/sti_plane.c
@@ -132,6 +132,60 @@ void sti_plane_init_property(struct sti_plane *plane,
plane->drm_plane.base.id, sti_plane_to_str(plane));
}
+int sti_plane_set_property(struct drm_plane *plane,
+ struct drm_property *property, uint64_t val)
+{
+ /*
+* Forked from drm_atomic_helper_plane_set_property().
+* Here we do not wait for vblank if the client is not atomic, so
+* DRM_IOCTL_MODE_OBJ_SETPROPERTY returns before vblank.
+*/
+ struct drm_atomic_state *state;
+ struct drm_plane_state *plane_state;
+ struct sti_private *private = plane->dev->dev_private;
+ int ret = 0;
+
+ state = drm_atomic_state_alloc(plane->dev);
+ if (!state)
+ return -ENOMEM;
+
+ /* ->set_property is always called with all locks held. */
+ state->acquire_ctx = plane->dev->mode_config.acquire_ctx;
+retry:
+ plane_state = drm_atomic_get_plane_state(state, plane);
+ if (IS_ERR(plane_state)) {
+ ret = PTR_ERR(plane_state);
+ goto fail;
+ }
+
+ ret = drm_atomic_plane_set_property(plane, plane_state,
+ property, val);
+ if (ret)
+ goto fail;
+
+ if (!private->filp->atomic)
+ state->legacy_cursor_update = true;
+
+ ret = drm_atomic_commit(state);
+ if (ret != 0)
+ goto fail;
+
+ /* Driver takes ownership of state on successful commit. */
+ return 0;
+fail:
+ if (ret == -EDEADLK)
+ goto backoff;
+
+ drm_atomic_state_free(state);
+
+ return ret;
+backoff:
+ drm_atomic_state_clear(state);
+ drm_atomic_legacy_backoff(state);
+
+ goto retry;
+}
+
int sti_plane_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
[PATCH 3/5] drm/sti: do not sync SETPLANE on vblank if not ATOMIC
If the client does not set the ATOMIC capability, do not wait for vblank
before returning an DRM_IOCTL_MODE_SETPLANE call.
In this way, a legacy framework (eg non-atomic Weston) can call several
SETPLANE within the same Vsync cycle.
This is implemented by setting the legacy_cursor_update flag, to behave
the same way as DRM_IOCTL_MODE_CURSOR (not vblank synced).
Change-Id: Ia241b6c88411c675bf589c17d4a44db6d02f669f
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_cursor.c | 4 +-
drivers/gpu/drm/sti/sti_gdp.c| 4 +-
drivers/gpu/drm/sti/sti_hqvdp.c | 4 +-
drivers/gpu/drm/sti/sti_plane.c | 144 +++
drivers/gpu/drm/sti/sti_plane.h | 8 +++
5 files changed, 158 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index cca75bd..ea0dbae 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -346,8 +346,8 @@ static int sti_cursor_late_register(struct drm_plane
*drm_plane)
}
static const struct drm_plane_funcs sti_cursor_plane_helpers_funcs = {
- .update_plane = drm_atomic_helper_update_plane,
- .disable_plane = drm_atomic_helper_disable_plane,
+ .update_plane = sti_plane_update_plane,
+ .disable_plane = sti_plane_disable_plane,
.destroy = sti_cursor_destroy,
.set_property = drm_atomic_helper_plane_set_property,
.reset = sti_plane_reset,
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index 81df309..a379bbe 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -882,8 +882,8 @@ static int sti_gdp_late_register(struct drm_plane
*drm_plane)
}
static const struct drm_plane_funcs sti_gdp_plane_helpers_funcs = {
- .update_plane = drm_atomic_helper_update_plane,
- .disable_plane = drm_atomic_helper_disable_plane,
+ .update_plane = sti_plane_update_plane,
+ .disable_plane = sti_plane_disable_plane,
.destroy = sti_gdp_destroy,
.set_property = drm_atomic_helper_plane_set_property,
.reset = sti_plane_reset,
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index f88130f..65ca43f 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1252,8 +1252,8 @@ static int sti_hqvdp_late_register(struct drm_plane
*drm_plane)
}
static const struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = {
- .update_plane = drm_atomic_helper_update_plane,
- .disable_plane = drm_atomic_helper_disable_plane,
+ .update_plane = sti_plane_update_plane,
+ .disable_plane = sti_plane_disable_plane,
.destroy = sti_hqvdp_destroy,
.set_property = drm_atomic_helper_plane_set_property,
.reset = sti_plane_reset,
diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
index ca4b371..22cf30d 100644
--- a/drivers/gpu/drm/sti/sti_plane.c
+++ b/drivers/gpu/drm/sti/sti_plane.c
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#include
@@ -130,3 +131,146 @@ void sti_plane_init_property(struct sti_plane *plane,
DRM_DEBUG_DRIVER("drm plane:%d mapped to %s\n",
plane->drm_plane.base.id, sti_plane_to_str(plane));
}
+
+int sti_plane_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
+{
+ /*
+* Forked from drm_atomic_helper_update_plane().
+* Here we do not wait for vblank if the client is not atomic, so
+* DRM_IOCTL_MODE_SETPLANE returns before vblank.
+*/
+
+ struct drm_atomic_state *state;
+ struct drm_plane_state *plane_state;
+ struct sti_private *private = plane->dev->dev_private;
+ int ret = 0;
+
+ state = drm_atomic_state_alloc(plane->dev);
+ if (!state)
+ return -ENOMEM;
+
+ state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+retry:
+ plane_state = drm_atomic_get_plane_state(state, plane);
+ if (IS_ERR(plane_state)) {
+ ret = PTR_ERR(plane_state);
+ goto fail;
+ }
+
+ ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
+ if (ret != 0)
+ goto fail;
+ drm_atomic_set_fb_for_plane(plane_state, fb);
+ plane_state->crtc_x = crtc_x;
+ plane_state->crtc_y = crtc_y;
+ plane_state->crtc_w = crtc_w;
+ plane_state->crtc_h = crtc_h;
+ plane_state->src_x = src_x;
+ plane_state->src_y = src_y;
+ plane_state->src_w = src_w;
+ plane_state->src_h = src_h;
+
+ if ((plane == crtc->cursor) || !private->filp->
[PATCH 2/5] drm/sti: add drm_file to sti_private
Store the drm_file *filp in sti_private, so the driver can access more
configuration information like the client capabilities.
Change-Id: Ib8f305f1a41b4fdfe56f80294cd79e5dc44433ee
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_drv.c | 10 ++
drivers/gpu/drm/sti/sti_drv.h | 3 +++
2 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index 9a9b9a2..63febf8 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -170,6 +170,15 @@ static int sti_atomic_check(struct drm_device *dev,
return ret;
}
+static int sti_drm_open(struct drm_device *ddev, struct drm_file *filp)
+{
+ struct sti_private *private = ddev->dev_private;
+
+ private->filp = filp;
+
+ return 0;
+}
+
static void sti_output_poll_changed(struct drm_device *ddev)
{
struct sti_private *private = ddev->dev_private;
@@ -226,6 +235,7 @@ static const struct file_operations sti_driver_fops = {
static struct drm_driver sti_driver = {
.driver_features = DRIVER_MODESET |
DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
+ .open = sti_drm_open,
.gem_free_object_unlocked = drm_gem_cma_free_object,
.gem_vm_ops = _gem_cma_vm_ops,
.dumb_create = drm_gem_cma_dumb_create,
diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h
index f9276cd..de22b0d 100644
--- a/drivers/gpu/drm/sti/sti_drv.h
+++ b/drivers/gpu/drm/sti/sti_drv.h
@@ -19,12 +19,15 @@ struct sti_tvout;
* @compo: compositor
* @plane_zorder_property: z-order property for CRTC planes
* @drm_dev: drm device
+ * @fbdev: framebuffer dev cma struct
+ * @drm_file: drm file private data
*/
struct sti_private {
struct sti_compositor *compo;
struct drm_property *plane_zorder_property;
struct drm_device *drm_dev;
struct drm_fbdev_cma *fbdev;
+ struct drm_file *filp;
};
extern struct platform_driver sti_tvout_driver;
--
2.7.4
[PATCH 1/5] drm/sti: use atomic_helper for commit
Since nonblocking atomic commits are now supported, the driver can
now use drm_atomic_helper_commit().
Change-Id: I3e49872b0dc9e79ca652bec7e5cd29d912c86382
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_drv.c | 83 +--
drivers/gpu/drm/sti/sti_drv.h | 6
2 files changed, 1 insertion(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index ff71e25..9a9b9a2 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -150,52 +150,6 @@ static void sti_drm_dbg_cleanup(struct drm_minor *minor)
1, minor);
}
-static void sti_atomic_schedule(struct sti_private *private,
- struct drm_atomic_state *state)
-{
- private->commit.state = state;
- schedule_work(>commit.work);
-}
-
-static void sti_atomic_complete(struct sti_private *private,
- struct drm_atomic_state *state)
-{
- struct drm_device *drm = private->drm_dev;
-
- /*
-* Everything below can be run asynchronously without the need to grab
-* any modeset locks at all under one condition: It must be guaranteed
-* that the asynchronous work has either been cancelled (if the driver
-* supports it, which at least requires that the framebuffers get
-* cleaned up with drm_atomic_helper_cleanup_planes()) or completed
-* before the new state gets committed on the software side with
-* drm_atomic_helper_swap_state().
-*
-* This scheme allows new atomic state updates to be prepared and
-* checked in parallel to the asynchronous completion of the previous
-* update. Which is important since compositors need to figure out the
-* composition of the next frame right after having submitted the
-* current layout.
-*/
-
- drm_atomic_helper_commit_modeset_disables(drm, state);
- drm_atomic_helper_commit_planes(drm, state, 0);
- drm_atomic_helper_commit_modeset_enables(drm, state);
-
- drm_atomic_helper_wait_for_vblanks(drm, state);
-
- drm_atomic_helper_cleanup_planes(drm, state);
- drm_atomic_state_put(state);
-}
-
-static void sti_atomic_work(struct work_struct *work)
-{
- struct sti_private *private = container_of(work,
- struct sti_private, commit.work);
-
- sti_atomic_complete(private, private->commit.state);
-}
-
static int sti_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
@@ -216,38 +170,6 @@ static int sti_atomic_check(struct drm_device *dev,
return ret;
}
-static int sti_atomic_commit(struct drm_device *drm,
-struct drm_atomic_state *state, bool nonblock)
-{
- struct sti_private *private = drm->dev_private;
- int err;
-
- err = drm_atomic_helper_prepare_planes(drm, state);
- if (err)
- return err;
-
- /* serialize outstanding nonblocking commits */
- mutex_lock(>commit.lock);
- flush_work(>commit.work);
-
- /*
-* This is the point of no return - everything below never fails except
-* when the hw goes bonghits. Which means we can commit the new state on
-* the software side now.
-*/
-
- drm_atomic_helper_swap_state(state, true);
-
- drm_atomic_state_get(state);
- if (nonblock)
- sti_atomic_schedule(private, state);
- else
- sti_atomic_complete(private, state);
-
- mutex_unlock(>commit.lock);
- return 0;
-}
-
static void sti_output_poll_changed(struct drm_device *ddev)
{
struct sti_private *private = ddev->dev_private;
@@ -271,7 +193,7 @@ static const struct drm_mode_config_funcs
sti_mode_config_funcs = {
.fb_create = drm_fb_cma_create,
.output_poll_changed = sti_output_poll_changed,
.atomic_check = sti_atomic_check,
- .atomic_commit = sti_atomic_commit,
+ .atomic_commit = drm_atomic_helper_commit,
};
static void sti_mode_config_init(struct drm_device *dev)
@@ -352,9 +274,6 @@ static int sti_init(struct drm_device *ddev)
dev_set_drvdata(ddev->dev, ddev);
private->drm_dev = ddev;
- mutex_init(>commit.lock);
- INIT_WORK(>commit.work, sti_atomic_work);
-
drm_mode_config_init(ddev);
sti_mode_config_init(ddev);
diff --git a/drivers/gpu/drm/sti/sti_drv.h b/drivers/gpu/drm/sti/sti_drv.h
index 78ebe5e..f9276cd 100644
--- a/drivers/gpu/drm/sti/sti_drv.h
+++ b/drivers/gpu/drm/sti/sti_drv.h
@@ -25,12 +25,6 @@ struct sti_private {
struct drm_property *plane_zorder_property;
struct drm_device *drm_dev;
struct drm_fbdev_cma *fbdev;
-
- struct {
- struct drm_atomic_state *state;
- struct work_struct work;
- struct mut
[PATCH 0/5] drm/sti: do not sync legacy IOCTL on vblank if not ATOMIC
These patches allow a legacy framework (eg non-atomic Weston) to call several SETPLANE within the same Vsync cycle. - [PATCH 1/5] drm/sti: use atomic_helper for commit - [PATCH 2/5] drm/sti: add drm_file to sti_private - [PATCH 3/5] drm/sti: do not sync SETPLANE on vblank if not ATOMIC - [PATCH 4/5] drm/sti: do not sync SETPROPERTY on vblank if not ATOMIC - [PATCH 5/5] drm/sti: do not check hw scaling if mode is not set Fabien
[PATCH 2/2] drm/sti: do not post HQVDP command if no update
Do not process update requests with unmodified parameters.
Since the HQVDP command queue is limited to 2, we shall take care of
not posting unneeded commands, which would abusively fill the command
queue leading to frame update skip.
This typically happens when the driver is called with legacy
(non-atomic) IOCTL : in that case atomic_update() is called multiple
times with the same parameters.
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_hqvdp.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index a547723..55cbaea 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1117,6 +1117,21 @@ static void sti_hqvdp_atomic_update(struct drm_plane
*drm_plane,
if (!crtc || !fb)
return;
+ if ((oldstate->fb == state->fb) &&
+ (oldstate->crtc_x == state->crtc_x) &&
+ (oldstate->crtc_y == state->crtc_y) &&
+ (oldstate->crtc_w == state->crtc_w) &&
+ (oldstate->crtc_h == state->crtc_h) &&
+ (oldstate->src_x == state->src_x) &&
+ (oldstate->src_y == state->src_y) &&
+ (oldstate->src_w == state->src_w) &&
+ (oldstate->src_h == state->src_h)) {
+ /* No change since last update, do not post cmd */
+ DRM_DEBUG_DRIVER("No change, not posting cmd\n");
+ plane->status = STI_PLANE_UPDATED;
+ return;
+ }
+
mode = >mode;
dst_x = state->crtc_x;
dst_y = state->crtc_y;
--
2.7.4
[PATCH 1/2] drm/sti: load XP70 firmware only once
When a plane is enabled, after having been disabled, do not reload XP70
firmware again, but only register VTG again
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_hqvdp.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index f88130f..a547723 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -332,6 +332,7 @@ struct sti_hqvdp_cmd {
* @hqvdp_cmd_paddr: physical address of hqvdp_cmd
* @vtg: vtg for main data path
* @xp70_initialized: true if xp70 is already initialized
+ * @vtg_registered:true if registered to VTG
*/
struct sti_hqvdp {
struct device *dev;
@@ -347,6 +348,7 @@ struct sti_hqvdp {
u32 hqvdp_cmd_paddr;
struct sti_vtg *vtg;
bool xp70_initialized;
+ bool vtg_registered;
};
#define to_sti_hqvdp(x) container_of(x, struct sti_hqvdp, plane)
@@ -771,7 +773,7 @@ static void sti_hqvdp_disable(struct sti_hqvdp *hqvdp)
DRM_ERROR("XP70 could not revert to idle\n");
hqvdp->plane.status = STI_PLANE_DISABLED;
- hqvdp->xp70_initialized = false;
+ hqvdp->vtg_registered = false;
}
/**
@@ -1064,10 +1066,11 @@ static int sti_hqvdp_atomic_check(struct drm_plane
*drm_plane,
return -EINVAL;
}
- if (!hqvdp->xp70_initialized) {
+ if (!hqvdp->xp70_initialized)
/* Start HQVDP XP70 coprocessor */
sti_hqvdp_start_xp70(hqvdp);
+ if (!hqvdp->vtg_registered) {
/* Prevent VTG shutdown */
if (clk_prepare_enable(hqvdp->clk_pix_main)) {
DRM_ERROR("Failed to prepare/enable pix main clk\n");
@@ -1081,6 +1084,7 @@ static int sti_hqvdp_atomic_check(struct drm_plane
*drm_plane,
DRM_ERROR("Cannot register VTG notifier\n");
return -EINVAL;
}
+ hqvdp->vtg_registered = true;
}
DRM_DEBUG_KMS("CRTC:%d (%s) drm plane:%d (%s)\n",
--
2.7.4
[PATCH 0/2] drm/sti: HQVDP plane improvements
These patches fix HQVDP plane performance issues - [PATCH 1/2] drm/sti: load XP70 firmware only once - [PATCH 2/2] drm/sti: do not post HQVDP command if no update Fabien
[PATCH 8/8] drm/sti: use valid video mode
In atomic mode the crtc_xxx (eg crtc_hdisplay) members of the mode structure may be unset before calling atomic_check/commit for planes. Instead of, use xxx members which are actually set. Signed-off-by: Fabien Dessenne --- drivers/gpu/drm/sti/sti_gdp.c | 8 drivers/gpu/drm/sti/sti_hqvdp.c | 8 drivers/gpu/drm/sti/sti_vid.c | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c index 824020f..3297f3b 100644 --- a/drivers/gpu/drm/sti/sti_gdp.c +++ b/drivers/gpu/drm/sti/sti_gdp.c @@ -631,8 +631,8 @@ static int sti_gdp_atomic_check(struct drm_plane *drm_plane, mode = _state->mode; dst_x = state->crtc_x; dst_y = state->crtc_y; - dst_w = clamp_val(state->crtc_w, 0, mode->crtc_hdisplay - dst_x); - dst_h = clamp_val(state->crtc_h, 0, mode->crtc_vdisplay - dst_y); + dst_w = clamp_val(state->crtc_w, 0, mode->hdisplay - dst_x); + dst_h = clamp_val(state->crtc_h, 0, mode->vdisplay - dst_y); /* src_x are in 16.16 format */ src_x = state->src_x >> 16; src_y = state->src_y >> 16; @@ -736,8 +736,8 @@ static void sti_gdp_atomic_update(struct drm_plane *drm_plane, mode = >mode; dst_x = state->crtc_x; dst_y = state->crtc_y; - dst_w = clamp_val(state->crtc_w, 0, mode->crtc_hdisplay - dst_x); - dst_h = clamp_val(state->crtc_h, 0, mode->crtc_vdisplay - dst_y); + dst_w = clamp_val(state->crtc_w, 0, mode->hdisplay - dst_x); + dst_h = clamp_val(state->crtc_h, 0, mode->vdisplay - dst_y); /* src_x are in 16.16 format */ src_x = state->src_x >> 16; src_y = state->src_y >> 16; diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index 9dd13fd..31fa7ae 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -1026,8 +1026,8 @@ static int sti_hqvdp_atomic_check(struct drm_plane *drm_plane, mode = _state->mode; dst_x = state->crtc_x; dst_y = state->crtc_y; - dst_w = clamp_val(state->crtc_w, 0, mode->crtc_hdisplay - dst_x); - dst_h = clamp_val(state->crtc_h, 0, mode->crtc_vdisplay - dst_y); + dst_w = clamp_val(state->crtc_w, 0, mode->hdisplay - dst_x); + dst_h = clamp_val(state->crtc_h, 0, mode->vdisplay - dst_y); /* src_x are in 16.16 format */ src_x = state->src_x >> 16; src_y = state->src_y >> 16; @@ -1115,8 +1115,8 @@ static void sti_hqvdp_atomic_update(struct drm_plane *drm_plane, mode = >mode; dst_x = state->crtc_x; dst_y = state->crtc_y; - dst_w = clamp_val(state->crtc_w, 0, mode->crtc_hdisplay - dst_x); - dst_h = clamp_val(state->crtc_h, 0, mode->crtc_vdisplay - dst_y); + dst_w = clamp_val(state->crtc_w, 0, mode->hdisplay - dst_x); + dst_h = clamp_val(state->crtc_h, 0, mode->vdisplay - dst_y); /* src_x are in 16.16 format */ src_x = state->src_x >> 16; src_y = state->src_y >> 16; diff --git a/drivers/gpu/drm/sti/sti_vid.c b/drivers/gpu/drm/sti/sti_vid.c index 47634a0..2ad5989 100644 --- a/drivers/gpu/drm/sti/sti_vid.c +++ b/drivers/gpu/drm/sti/sti_vid.c @@ -142,8 +142,8 @@ void sti_vid_commit(struct sti_vid *vid, struct drm_display_mode *mode = >mode; int dst_x = state->crtc_x; int dst_y = state->crtc_y; - int dst_w = clamp_val(state->crtc_w, 0, mode->crtc_hdisplay - dst_x); - int dst_h = clamp_val(state->crtc_h, 0, mode->crtc_vdisplay - dst_y); + int dst_w = clamp_val(state->crtc_w, 0, mode->hdisplay - dst_x); + int dst_h = clamp_val(state->crtc_h, 0, mode->vdisplay - dst_y); int src_h = state->src_h >> 16; u32 val, ydo, xdo, yds, xds; -- 1.9.1
[PATCH 7/8] drm/sti: in crtc_atomic_flush, enable only planes of this crtc
crtc_atomic_flush performs some additional processing, like plane
enable at mixer level.
Enable only the planes attached to the CRTC.
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_crtc.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c
index 96afe68..bc1b186 100644
--- a/drivers/gpu/drm/sti/sti_crtc.c
+++ b/drivers/gpu/drm/sti/sti_crtc.c
@@ -165,6 +165,10 @@ static void sti_crtc_atomic_flush(struct drm_crtc *crtc,
switch (plane->status) {
case STI_PLANE_UPDATED:
+ /* ignore update for other CRTC */
+ if (p->state->crtc != crtc)
+ continue;
+
/* update planes tag as updated */
DRM_DEBUG_DRIVER("update plane %s\n",
sti_plane_to_str(plane));
--
1.9.1
[PATCH 6/8] drm/sti: use vtg array instead of vtg_main/aux
This is more generic and more consistent with the other members of the
sti_compositor struct.
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_compositor.c | 4 ++--
drivers/gpu/drm/sti/sti_compositor.h | 6 ++
drivers/gpu/drm/sti/sti_crtc.c | 12 +---
drivers/gpu/drm/sti/sti_gdp.c| 3 +--
4 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_compositor.c
b/drivers/gpu/drm/sti/sti_compositor.c
index f61c16d..f0c6f0a 100644
--- a/drivers/gpu/drm/sti/sti_compositor.c
+++ b/drivers/gpu/drm/sti/sti_compositor.c
@@ -268,12 +268,12 @@ static int sti_compositor_probe(struct platform_device
*pdev)
vtg_np = of_parse_phandle(pdev->dev.of_node, "st,vtg", 0);
if (vtg_np)
- compo->vtg_main = of_vtg_find(vtg_np);
+ compo->vtg[STI_MIXER_MAIN] = of_vtg_find(vtg_np);
of_node_put(vtg_np);
vtg_np = of_parse_phandle(pdev->dev.of_node, "st,vtg", 1);
if (vtg_np)
- compo->vtg_aux = of_vtg_find(vtg_np);
+ compo->vtg[STI_MIXER_AUX] = of_vtg_find(vtg_np);
of_node_put(vtg_np);
platform_set_drvdata(pdev, compo);
diff --git a/drivers/gpu/drm/sti/sti_compositor.h
b/drivers/gpu/drm/sti/sti_compositor.h
index 177c57b..c9e7e3b 100644
--- a/drivers/gpu/drm/sti/sti_compositor.h
+++ b/drivers/gpu/drm/sti/sti_compositor.h
@@ -60,8 +60,7 @@ struct sti_compositor_data {
* @rst_aux: reset control of the aux path
* @mixer: array of mixers
* @vid: array of vids
- * @vtg_main: vtg for main data path
- * @vtg_aux: vtg for auxillary data path
+ * @vtg: array of vtgs
* @vtg_vblank_nb: array of callbacks for VTG VSYNC notification
*/
struct sti_compositor {
@@ -76,8 +75,7 @@ struct sti_compositor {
struct reset_control *rst_aux;
struct sti_mixer *mixer[STI_MAX_MIXER];
struct sti_vid *vid[STI_MAX_VID];
- struct sti_vtg *vtg_main;
- struct sti_vtg *vtg_aux;
+ struct sti_vtg *vtg[STI_MAX_MIXER];
struct notifier_block vtg_vblank_nb[STI_MAX_MIXER];
};
diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c
index 2f41cbe..96afe68 100644
--- a/drivers/gpu/drm/sti/sti_crtc.c
+++ b/drivers/gpu/drm/sti/sti_crtc.c
@@ -86,8 +86,7 @@ sti_crtc_mode_set(struct drm_crtc *crtc, struct
drm_display_mode *mode)
goto pix_error;
}
- sti_vtg_set_config(mixer->id == STI_MIXER_MAIN ?
- compo->vtg_main : compo->vtg_aux, >mode);
+ sti_vtg_set_config(compo->vtg[mixer->id], >mode);
if (sti_mixer_active_video_area(mixer, >mode)) {
DRM_ERROR("Can't set active video area\n");
@@ -297,12 +296,11 @@ int sti_crtc_enable_vblank(struct drm_device *dev,
unsigned int pipe)
struct sti_compositor *compo = dev_priv->compo;
struct notifier_block *vtg_vblank_nb = >vtg_vblank_nb[pipe];
struct drm_crtc *crtc = >mixer[pipe]->drm_crtc;
+ struct sti_vtg *vtg = compo->vtg[pipe];
DRM_DEBUG_DRIVER("\n");
- if (sti_vtg_register_client(pipe == STI_MIXER_MAIN ?
- compo->vtg_main : compo->vtg_aux,
- vtg_vblank_nb, crtc)) {
+ if (sti_vtg_register_client(vtg, vtg_vblank_nb, crtc)) {
DRM_ERROR("Cannot register VTG notifier\n");
return -EINVAL;
}
@@ -316,11 +314,11 @@ void sti_crtc_disable_vblank(struct drm_device *drm_dev,
unsigned int pipe)
struct sti_compositor *compo = priv->compo;
struct notifier_block *vtg_vblank_nb = >vtg_vblank_nb[pipe];
struct drm_crtc *crtc = >mixer[pipe]->drm_crtc;
+ struct sti_vtg *vtg = compo->vtg[pipe];
DRM_DEBUG_DRIVER("\n");
- if (sti_vtg_unregister_client(pipe == STI_MIXER_MAIN ?
- compo->vtg_main : compo->vtg_aux, vtg_vblank_nb))
+ if (sti_vtg_unregister_client(vtg, vtg_vblank_nb))
DRM_DEBUG_DRIVER("Warning: cannot unregister VTG notifier\n");
/* free the resources of the pending requests */
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index d5f7b18..824020f 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -653,8 +653,7 @@ static int sti_gdp_atomic_check(struct drm_plane *drm_plane,
if (!gdp->vtg) {
/* Register gdp callback */
- gdp->vtg = mixer->id == STI_MIXER_MAIN ?
- compo->vtg_main : compo->vtg_aux;
+ gdp->vtg = compo->vtg[mixer->id];
if (sti_vtg_register_client(gdp->vtg,
>vtg_field_nb, crtc)) {
DRM_ERROR("Cannot register VTG notifier\n");
--
1.9.1
[PATCH 5/8] drm/sti: use different notifier_block for each pipe
Each pipe shall have its own notifier block to manage the vblank event.
This fixes issues where a client registered on given pipe is later
abusively notified of events on the other pipe.
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_compositor.c | 4 +++-
drivers/gpu/drm/sti/sti_compositor.h | 4 ++--
drivers/gpu/drm/sti/sti_crtc.c | 8
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_compositor.c
b/drivers/gpu/drm/sti/sti_compositor.c
index 134201e..f61c16d 100644
--- a/drivers/gpu/drm/sti/sti_compositor.c
+++ b/drivers/gpu/drm/sti/sti_compositor.c
@@ -201,6 +201,7 @@ static int sti_compositor_probe(struct platform_device
*pdev)
struct device_node *vtg_np;
struct sti_compositor *compo;
struct resource *res;
+ unsigned int i;
compo = devm_kzalloc(dev, sizeof(*compo), GFP_KERNEL);
if (!compo) {
@@ -208,7 +209,8 @@ static int sti_compositor_probe(struct platform_device
*pdev)
return -ENOMEM;
}
compo->dev = dev;
- compo->vtg_vblank_nb.notifier_call = sti_crtc_vblank_cb;
+ for (i = 0; i < STI_MAX_MIXER; i++)
+ compo->vtg_vblank_nb[i].notifier_call = sti_crtc_vblank_cb;
/* populate data structure depending on compatibility */
BUG_ON(!of_match_node(compositor_of_match, np)->data);
diff --git a/drivers/gpu/drm/sti/sti_compositor.h
b/drivers/gpu/drm/sti/sti_compositor.h
index 2ef..177c57b 100644
--- a/drivers/gpu/drm/sti/sti_compositor.h
+++ b/drivers/gpu/drm/sti/sti_compositor.h
@@ -62,7 +62,7 @@ struct sti_compositor_data {
* @vid: array of vids
* @vtg_main: vtg for main data path
* @vtg_aux: vtg for auxillary data path
- * @vtg_vblank_nb: callback for VTG VSYNC notification
+ * @vtg_vblank_nb: array of callbacks for VTG VSYNC notification
*/
struct sti_compositor {
struct device *dev;
@@ -78,7 +78,7 @@ struct sti_compositor {
struct sti_vid *vid[STI_MAX_VID];
struct sti_vtg *vtg_main;
struct sti_vtg *vtg_aux;
- struct notifier_block vtg_vblank_nb;
+ struct notifier_block vtg_vblank_nb[STI_MAX_MIXER];
};
int sti_compositor_debufs_init(struct sti_compositor *compo,
diff --git a/drivers/gpu/drm/sti/sti_crtc.c b/drivers/gpu/drm/sti/sti_crtc.c
index c7d734d..2f41cbe 100644
--- a/drivers/gpu/drm/sti/sti_crtc.c
+++ b/drivers/gpu/drm/sti/sti_crtc.c
@@ -244,8 +244,7 @@ static int sti_crtc_set_property(struct drm_crtc *crtc,
int sti_crtc_vblank_cb(struct notifier_block *nb,
unsigned long event, void *data)
{
- struct sti_compositor *compo =
- container_of(nb, struct sti_compositor, vtg_vblank_nb);
+ struct sti_compositor *compo;
struct drm_crtc *crtc = data;
struct sti_mixer *mixer;
unsigned long flags;
@@ -254,6 +253,7 @@ int sti_crtc_vblank_cb(struct notifier_block *nb,
priv = crtc->dev->dev_private;
pipe = drm_crtc_index(crtc);
+ compo = container_of(nb, struct sti_compositor, vtg_vblank_nb[pipe]);
mixer = compo->mixer[pipe];
if ((event != VTG_TOP_FIELD_EVENT) &&
@@ -295,7 +295,7 @@ int sti_crtc_enable_vblank(struct drm_device *dev, unsigned
int pipe)
{
struct sti_private *dev_priv = dev->dev_private;
struct sti_compositor *compo = dev_priv->compo;
- struct notifier_block *vtg_vblank_nb = >vtg_vblank_nb;
+ struct notifier_block *vtg_vblank_nb = >vtg_vblank_nb[pipe];
struct drm_crtc *crtc = >mixer[pipe]->drm_crtc;
DRM_DEBUG_DRIVER("\n");
@@ -314,7 +314,7 @@ void sti_crtc_disable_vblank(struct drm_device *drm_dev,
unsigned int pipe)
{
struct sti_private *priv = drm_dev->dev_private;
struct sti_compositor *compo = priv->compo;
- struct notifier_block *vtg_vblank_nb = >vtg_vblank_nb;
+ struct notifier_block *vtg_vblank_nb = >vtg_vblank_nb[pipe];
struct drm_crtc *crtc = >mixer[pipe]->drm_crtc;
DRM_DEBUG_DRIVER("\n");
--
1.9.1
[PATCH 4/8] drm/sti: fix atomic_disable check
When a drm_plane is being disabled, its ->crtc member is set to NULL
before the .atomic_disable() func is called.
To get the crtc of the plane, read old_state->crtc instead of
drm_plane->crtc
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_cursor.c | 6 +++---
drivers/gpu/drm/sti/sti_gdp.c| 6 +++---
drivers/gpu/drm/sti/sti_hqvdp.c | 6 +++---
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index 3b53f7f..3a8b656 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -309,15 +309,15 @@ static void sti_cursor_atomic_disable(struct drm_plane
*drm_plane,
{
struct sti_plane *plane = to_sti_plane(drm_plane);
- if (!drm_plane->crtc) {
+ if (!oldstate->crtc) {
DRM_DEBUG_DRIVER("drm plane:%d not enabled\n",
drm_plane->base.id);
return;
}
DRM_DEBUG_DRIVER("CRTC:%d (%s) drm plane:%d (%s)\n",
-drm_plane->crtc->base.id,
-sti_mixer_to_str(to_sti_mixer(drm_plane->crtc)),
+oldstate->crtc->base.id,
+sti_mixer_to_str(to_sti_mixer(oldstate->crtc)),
drm_plane->base.id, sti_plane_to_str(plane));
plane->status = STI_PLANE_DISABLING;
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index 19052c4..d5f7b18 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -858,15 +858,15 @@ static void sti_gdp_atomic_disable(struct drm_plane
*drm_plane,
/* restore possible crtcs value with the initial value */
drm_plane->possible_crtcs = plane->init_possible_crtcs;
- if (!drm_plane->crtc) {
+ if (!oldstate->crtc) {
DRM_DEBUG_DRIVER("drm plane:%d not enabled\n",
drm_plane->base.id);
return;
}
DRM_DEBUG_DRIVER("CRTC:%d (%s) drm plane:%d (%s)\n",
-drm_plane->crtc->base.id,
-sti_mixer_to_str(to_sti_mixer(drm_plane->crtc)),
+oldstate->crtc->base.id,
+sti_mixer_to_str(to_sti_mixer(oldstate->crtc)),
drm_plane->base.id, sti_plane_to_str(plane));
plane->status = STI_PLANE_DISABLING;
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index a222b2e..9dd13fd 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1214,15 +1214,15 @@ static void sti_hqvdp_atomic_disable(struct drm_plane
*drm_plane,
{
struct sti_plane *plane = to_sti_plane(drm_plane);
- if (!drm_plane->crtc) {
+ if (!oldstate->crtc) {
DRM_DEBUG_DRIVER("drm plane:%d not enabled\n",
drm_plane->base.id);
return;
}
DRM_DEBUG_DRIVER("CRTC:%d (%s) drm plane:%d (%s)\n",
-drm_plane->crtc->base.id,
-sti_mixer_to_str(to_sti_mixer(drm_plane->crtc)),
+oldstate->crtc->base.id,
+sti_mixer_to_str(to_sti_mixer(oldstate->crtc)),
drm_plane->base.id, sti_plane_to_str(plane));
plane->status = STI_PLANE_DISABLING;
--
1.9.1
[PATCH 3/8] drm/sti: run gdp init sequence only once
Do not rely on plane->status to define whether this is the first update
but rather check for gdp->vtg.
This avoids multiple and unwanted calls to sti_vtg_register_client()
which breaks the kernel scheduler.
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_gdp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index 3d48e5e..19052c4 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -463,6 +463,7 @@ static void sti_gdp_disable(struct sti_gdp *gdp)
clk_disable_unprepare(gdp->clk_pix);
gdp->plane.status = STI_PLANE_DISABLED;
+ gdp->vtg = NULL;
}
/**
@@ -614,7 +615,6 @@ static int sti_gdp_atomic_check(struct drm_plane *drm_plane,
struct drm_crtc *crtc = state->crtc;
struct sti_compositor *compo = dev_get_drvdata(gdp->dev);
struct drm_framebuffer *fb = state->fb;
- bool first_prepare = plane->status == STI_PLANE_DISABLED ? true : false;
struct drm_crtc_state *crtc_state;
struct sti_mixer *mixer;
struct drm_display_mode *mode;
@@ -651,7 +651,7 @@ static int sti_gdp_atomic_check(struct drm_plane *drm_plane,
return -EINVAL;
}
- if (first_prepare) {
+ if (!gdp->vtg) {
/* Register gdp callback */
gdp->vtg = mixer->id == STI_MIXER_MAIN ?
compo->vtg_main : compo->vtg_aux;
--
1.9.1
[PATCH 2/8] drm/sti: run hqvdp init sequence only once
Do not rely on plane->status to define whether this is the first update
but rather check for hqvdp->xp70_initialized bit status.
This avoids multiple and unwanted calls to sti_vtg_register_client()
which breaks the kernel scheduler.
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_hqvdp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index b5ee783..a222b2e 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -770,6 +770,7 @@ static void sti_hqvdp_disable(struct sti_hqvdp *hqvdp)
DRM_ERROR("XP70 could not revert to idle\n");
hqvdp->plane.status = STI_PLANE_DISABLED;
+ hqvdp->xp70_initialized = false;
}
/**
@@ -1012,7 +1013,6 @@ static int sti_hqvdp_atomic_check(struct drm_plane
*drm_plane,
struct sti_hqvdp *hqvdp = to_sti_hqvdp(plane);
struct drm_crtc *crtc = state->crtc;
struct drm_framebuffer *fb = state->fb;
- bool first_prepare = plane->status == STI_PLANE_DISABLED ? true : false;
struct drm_crtc_state *crtc_state;
struct drm_display_mode *mode;
int dst_x, dst_y, dst_w, dst_h;
@@ -1063,7 +1063,7 @@ static int sti_hqvdp_atomic_check(struct drm_plane
*drm_plane,
return -EINVAL;
}
- if (first_prepare) {
+ if (!hqvdp->xp70_initialized) {
/* Start HQVDP XP70 coprocessor */
sti_hqvdp_start_xp70(hqvdp);
--
1.9.1
[PATCH 1/8] drm/sti: fix debug logs
Add some missing \n in logs.
Signed-off-by: Fabien Dessenne
---
drivers/gpu/drm/sti/sti_gdp.c | 2 +-
drivers/gpu/drm/sti/sti_hda.c | 4 ++--
drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index f7cd671..3d48e5e 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -819,7 +819,7 @@ static void sti_gdp_atomic_update(struct drm_plane
*drm_plane,
if (!curr_list) {
/* First update or invalid node should directly write in the
* hw register */
- DRM_DEBUG_DRIVER("%s first update (or invalid node)",
+ DRM_DEBUG_DRIVER("%s first update (or invalid node)\n",
sti_plane_to_str(plane));
writel(gdp->is_curr_top ?
diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
index 8505569..a225c4d 100644
--- a/drivers/gpu/drm/sti/sti_hda.c
+++ b/drivers/gpu/drm/sti/sti_hda.c
@@ -313,7 +313,7 @@ static void hda_enable_hd_dacs(struct sti_hda *hda, bool
enable)
mask = DAC_CFG_HD_HZUVW_OFF_MASK;
break;
default:
- DRM_INFO("Video DACS control register not supported!");
+ DRM_INFO("Video DACS control register not supported\n");
return;
}
@@ -362,7 +362,7 @@ static void hda_dbg_video_dacs_ctrl(struct seq_file *s,
void __iomem *reg)
mask = DAC_CFG_HD_HZUVW_OFF_MASK;
break;
default:
- DRM_DEBUG_DRIVER("Warning: DACS ctrl register not supported!");
+ DRM_DEBUG_DRIVER("Warning: DACS ctrl register not supported\n");
return;
}
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index d850dda..1f9e7b4 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -203,7 +203,7 @@ static irqreturn_t hdmi_irq_thread(int irq, void *arg)
/* Audio FIFO underrun IRQ */
if (hdmi->irq_status & HDMI_INT_AUDIO_FIFO_XRUN)
- DRM_INFO("Warning: audio FIFO underrun occurs!");
+ DRM_INFO("Warning: audio FIFO underrun occurs!\n");
return IRQ_HANDLED;
}
--
1.9.1
[PATCH 0/8] drm/sti: atomic bug fixes
This serie provides with bug fixes found while testing drm/sti with an atomic version of weston [PATCH 1/8] drm/sti: fix debug logs [PATCH 2/8] drm/sti: run hqvdp init sequence only once [PATCH 3/8] drm/sti: run gdp init sequence only once [PATCH 4/8] drm/sti: fix atomic_disable check [PATCH 5/8] drm/sti: use different notifier_block for each pipe [PATCH 6/8] drm/sti: use vtg array instead of vtg_main/aux [PATCH 7/8] drm/sti: in crtc_atomic_flush, enable only planes of this [PATCH 8/8] drm/sti: use valid video mode
[PATCH] drm: Set depth and bpp for XRGB4444 family formats
Hi Laurent, Daniel talked about a planned reorg from you around DRM formats (or something like this) and I have proposed to fix some missing parts with the XRGB444 formats. Can you let us know if you can consider this fix as part of your reorg? If you can't what would be the best option to get this fixed? Fabien On 08/10/2016 05:26 PM, Fabien DESSENNE wrote: > > On 08/10/2016 04:12 PM, Daniel Vetter wrote: >> On Wed, Aug 10, 2016 at 01:04:54PM +0200, Fabien DESSENNE wrote: >>> On 08/10/2016 12:35 PM, Daniel Vetter wrote: >>>> On Wed, Aug 10, 2016 at 11:21:56AM +0200, Fabien Dessenne wrote: >>>>> These pixel formats are supported by format_check() from >>>>> drm_crtc.c, so >>>>> provide there depth and bpp. >>>>> >>>>> Signed-off-by: Fabien Dessenne >>>> Why? >>> At least for consistency between format_check() and >>> drm_fb_get_bpp_depth(). >> fb_get_bpp_depth is kinda legacy, the recommended way is to have a >> switch >> statement in your driver that directly decodes DRM_FORMAT_* into driver >> register values. The inconsistency is intentional since just looking at >> bpp and depth removes a lot of information. > I can understand that fb_get_bpp_depth() shall not be called by (new) > drivers. > But it is also called by the core part through drm_format_plane_cpp() > which is not a legacy interface and is used across many drivers (I > think that this is a recent change initiated by Ville). >> Probably a better patch would be to update the kerneldoc for these >> functions. > > For the time being we have an issue with the current drivers that use > the DRM formats and updating kerneldoc won't fix them >> Also Laurent is working on a complete reorg of all this. > > Looping Laurent who may have a different opinion, or plan to consider > the DRM formats in its rework. > > Fabien >> -Daniel >> >>>> Who's going to use this? >>> For the time being, I can see 9 drivers that make more or less use of >>> this format (amd, atmel-hlcdc, exynos, fsl-dcu, imx, omapdrm, radeon, >>> rcar-du, sti). >>> In top of that I am working on a new driver that actually needs this >>> format, and that does not work without this patch. >>>> -Daniel >>>> >>>>> --- >>>>>drivers/gpu/drm/drm_fourcc.c | 11 +++ >>>>>1 file changed, 11 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_fourcc.c >>>>> b/drivers/gpu/drm/drm_fourcc.c >>>>> index 0645c85..aa8c909 100644 >>>>> --- a/drivers/gpu/drm/drm_fourcc.c >>>>> +++ b/drivers/gpu/drm/drm_fourcc.c >>>>> @@ -80,6 +80,17 @@ void drm_fb_get_bpp_depth(uint32_t format, >>>>> unsigned int *depth, >>>>>*depth = 8; >>>>>*bpp = 8; >>>>>break; >>>>> +case DRM_FORMAT_XRGB: >>>>> +case DRM_FORMAT_XBGR: >>>>> +case DRM_FORMAT_RGBX: >>>>> +case DRM_FORMAT_BGRX: >>>>> +case DRM_FORMAT_ARGB: >>>>> +case DRM_FORMAT_ABGR: >>>>> +case DRM_FORMAT_RGBA: >>>>> +case DRM_FORMAT_BGRA: >>>>> +*depth = 12; >>>>> +*bpp = 16; >>>>> +break; >>>>>case DRM_FORMAT_XRGB1555: >>>>>case DRM_FORMAT_XBGR1555: >>>>>case DRM_FORMAT_RGBX5551: >>>>> -- >>>>> 1.9.1 >>>>> >>>>> ___ >>>>> dri-devel mailing list >>>>> dri-devel at lists.freedesktop.org >>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >
[PATCH] drm: Set depth and bpp for XRGB4444 family formats
On 08/10/2016 04:12 PM, Daniel Vetter wrote: > On Wed, Aug 10, 2016 at 01:04:54PM +0200, Fabien DESSENNE wrote: >> On 08/10/2016 12:35 PM, Daniel Vetter wrote: >>> On Wed, Aug 10, 2016 at 11:21:56AM +0200, Fabien Dessenne wrote: >>>> These pixel formats are supported by format_check() from drm_crtc.c, so >>>> provide there depth and bpp. >>>> >>>> Signed-off-by: Fabien Dessenne >>> Why? >> At least for consistency between format_check() and drm_fb_get_bpp_depth(). > fb_get_bpp_depth is kinda legacy, the recommended way is to have a switch > statement in your driver that directly decodes DRM_FORMAT_* into driver > register values. The inconsistency is intentional since just looking at > bpp and depth removes a lot of information. I can understand that fb_get_bpp_depth() shall not be called by (new) drivers. But it is also called by the core part through drm_format_plane_cpp() which is not a legacy interface and is used across many drivers (I think that this is a recent change initiated by Ville). > Probably a better patch would be to update the kerneldoc for these > functions. For the time being we have an issue with the current drivers that use the DRM formats and updating kerneldoc won't fix them > Also Laurent is working on a complete reorg of all this. Looping Laurent who may have a different opinion, or plan to consider the DRM formats in its rework. Fabien > -Daniel > >>> Who's going to use this? >> For the time being, I can see 9 drivers that make more or less use of >> this format (amd, atmel-hlcdc, exynos, fsl-dcu, imx, omapdrm, radeon, >> rcar-du, sti). >> In top of that I am working on a new driver that actually needs this >> format, and that does not work without this patch. >>> -Daniel >>> >>>> --- >>>>drivers/gpu/drm/drm_fourcc.c | 11 +++ >>>>1 file changed, 11 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c >>>> index 0645c85..aa8c909 100644 >>>> --- a/drivers/gpu/drm/drm_fourcc.c >>>> +++ b/drivers/gpu/drm/drm_fourcc.c >>>> @@ -80,6 +80,17 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int >>>> *depth, >>>>*depth = 8; >>>>*bpp = 8; >>>>break; >>>> + case DRM_FORMAT_XRGB: >>>> + case DRM_FORMAT_XBGR: >>>> + case DRM_FORMAT_RGBX: >>>> + case DRM_FORMAT_BGRX: >>>> + case DRM_FORMAT_ARGB: >>>> + case DRM_FORMAT_ABGR: >>>> + case DRM_FORMAT_RGBA: >>>> + case DRM_FORMAT_BGRA: >>>> + *depth = 12; >>>> + *bpp = 16; >>>> + break; >>>>case DRM_FORMAT_XRGB1555: >>>>case DRM_FORMAT_XBGR1555: >>>>case DRM_FORMAT_RGBX5551: >>>> -- >>>> 1.9.1 >>>> >>>> ___ >>>> dri-devel mailing list >>>> dri-devel at lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Set depth and bpp for XRGB4444 family formats
On 08/10/2016 12:35 PM, Daniel Vetter wrote: > On Wed, Aug 10, 2016 at 11:21:56AM +0200, Fabien Dessenne wrote: >> These pixel formats are supported by format_check() from drm_crtc.c, so >> provide there depth and bpp. >> >> Signed-off-by: Fabien Dessenne > Why? At least for consistency between format_check() and drm_fb_get_bpp_depth(). > Who's going to use this? For the time being, I can see 9 drivers that make more or less use of this format (amd, atmel-hlcdc, exynos, fsl-dcu, imx, omapdrm, radeon, rcar-du, sti). In top of that I am working on a new driver that actually needs this format, and that does not work without this patch. > -Daniel > >> --- >> drivers/gpu/drm/drm_fourcc.c | 11 +++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c >> index 0645c85..aa8c909 100644 >> --- a/drivers/gpu/drm/drm_fourcc.c >> +++ b/drivers/gpu/drm/drm_fourcc.c >> @@ -80,6 +80,17 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int >> *depth, >> *depth = 8; >> *bpp = 8; >> break; >> +case DRM_FORMAT_XRGB: >> +case DRM_FORMAT_XBGR: >> +case DRM_FORMAT_RGBX: >> +case DRM_FORMAT_BGRX: >> +case DRM_FORMAT_ARGB: >> +case DRM_FORMAT_ABGR: >> +case DRM_FORMAT_RGBA: >> +case DRM_FORMAT_BGRA: >> +*depth = 12; >> +*bpp = 16; >> +break; >> case DRM_FORMAT_XRGB1555: >> case DRM_FORMAT_XBGR1555: >> case DRM_FORMAT_RGBX5551: >> -- >> 1.9.1 >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Set depth and bpp for XRGB4444 family formats
These pixel formats are supported by format_check() from drm_crtc.c, so provide there depth and bpp. Signed-off-by: Fabien Dessenne --- drivers/gpu/drm/drm_fourcc.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index 0645c85..aa8c909 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -80,6 +80,17 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, *depth = 8; *bpp = 8; break; + case DRM_FORMAT_XRGB: + case DRM_FORMAT_XBGR: + case DRM_FORMAT_RGBX: + case DRM_FORMAT_BGRX: + case DRM_FORMAT_ARGB: + case DRM_FORMAT_ABGR: + case DRM_FORMAT_RGBA: + case DRM_FORMAT_BGRA: + *depth = 12; + *bpp = 16; + break; case DRM_FORMAT_XRGB1555: case DRM_FORMAT_XBGR1555: case DRM_FORMAT_RGBX5551: -- 1.9.1
[PATCH] drm: fix pixel size of NV12 / NV16 pixel formats
With NV12, NV21, NV16 and NV61 formats, the size of one pixel from the UV plane (plane #1) is one byte. Indeed, these pixel formats use 4:2:x chroma subsampling: the chroma pixels are sampled at half the luma: for 2 pixels, there are 1 Cb + 1 Cr = 2 bytes. So for plane #1, the correct size is actually 1 byte per pixel. Signed-off-by: Fabien Dessenne --- drivers/gpu/drm/drm_crtc.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 3313f7e..572c6fa 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5618,10 +5618,6 @@ int drm_format_plane_cpp(uint32_t format, int plane) case DRM_FORMAT_UYVY: case DRM_FORMAT_VYUY: return 2; - case DRM_FORMAT_NV12: - case DRM_FORMAT_NV21: - case DRM_FORMAT_NV16: - case DRM_FORMAT_NV61: case DRM_FORMAT_NV24: case DRM_FORMAT_NV42: return plane ? 2 : 1; @@ -5635,6 +5631,10 @@ int drm_format_plane_cpp(uint32_t format, int plane) case DRM_FORMAT_YVU422: case DRM_FORMAT_YUV444: case DRM_FORMAT_YVU444: + case DRM_FORMAT_NV12: + case DRM_FORMAT_NV21: + case DRM_FORMAT_NV16: + case DRM_FORMAT_NV61: return 1; default: drm_fb_get_bpp_depth(format, , ); -- 1.9.1
[PATCH 0/2] drm/sti: support of interlaced content with Bottom 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 >&g
[PATCH 0/2] drm/sti: support of interlaced content with Bottom 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. >>>
[PATCH 0/2] drm/sti: support of interlaced content with Bottom 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. >
[PATCH] drm/crtc-helper: use drm_framebuffer flags
Hi, Can anyone review this patch ? Thanks for your time. Fabien > -Original Message- > From: Fabien DESSENNE [mailto:fabien.dessenne at st.com] > Sent: mardi 1 juillet 2014 14:41 > To: dri-devel at lists.freedesktop.org > Cc: Benjamin Gaignard; Vincent ABRIOU; Fabien DESSENNE > Subject: [PATCH] drm/crtc-helper: use drm_framebuffer flags > > The "flags" parameter of the DRM_IOCTL_MODE_ADDFB2 ioctl must be > propagated and used by the driver. > The only possible value of flags is DRM_MODE_FB_INTERLACED. > > Signed-off-by: Fabien Dessenne > Reviewed-by: Benjamin GAIGNARD > --- > drivers/gpu/drm/drm_crtc_helper.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > b/drivers/gpu/drm/drm_crtc_helper.c > index 23500c0..5974489 100644 > --- a/drivers/gpu/drm/drm_crtc_helper.c > +++ b/drivers/gpu/drm/drm_crtc_helper.c > @@ -966,6 +966,7 @@ int drm_helper_mode_fill_fb_struct(struct > drm_framebuffer *fb, > drm_fb_get_bpp_depth(mode_cmd->pixel_format, >depth, > >bits_per_pixel); > fb->pixel_format = mode_cmd->pixel_format; > + fb->flags = mode_cmd->flags; > > return 0; > } > -- > 1.9.1
[PATCH] drm/crtc-helper: use drm_framebuffer flags
The "flags" parameter of the DRM_IOCTL_MODE_ADDFB2 ioctl must be propagated and used by the driver. The only possible value of flags is DRM_MODE_FB_INTERLACED. Signed-off-by: Fabien Dessenne Reviewed-by: Benjamin GAIGNARD --- drivers/gpu/drm/drm_crtc_helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 23500c0..5974489 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -966,6 +966,7 @@ int drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb, drm_fb_get_bpp_depth(mode_cmd->pixel_format, >depth, >bits_per_pixel); fb->pixel_format = mode_cmd->pixel_format; + fb->flags = mode_cmd->flags; return 0; } -- 1.9.1
[PATCH] drm/crtc-helper: use drm_framebuffer flags
The "flags" parameter of the DRM_IOCTL_MODE_ADDFB2 ioctl must be propagated and used by the driver. The only possible value of flags is DRM_MODE_FB_INTERLACED. Change-Id: I989c01b1e6eef753eb004a5ac876665ea8ab0da6 Signed-off-by: Fabien Dessenne Change-Id: I2350ad8bd1553a4a7388e8d8b7733e65f1e8caef Reviewed-on: https://gerrit.st.com/8141 Reviewed-by: Benjamin GAIGNARD --- drivers/gpu/drm/drm_crtc_helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 23500c0..5974489 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -966,6 +966,7 @@ int drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb, drm_fb_get_bpp_depth(mode_cmd->pixel_format, >depth, >bits_per_pixel); fb->pixel_format = mode_cmd->pixel_format; + fb->flags = mode_cmd->flags; return 0; } -- 1.9.1
[PATCH] modetest: consider supported formats before selecting a DRM plane
Can anyone review this patch ?
Thanks for your time.
Fabien
> -Original Message-
> From: Fabien DESSENNE [mailto:fabien.dessenne at st.com]
> Sent: vendredi 28 mars 2014 11:16
> To: dri-devel at lists.freedesktop.org
> Cc: Benjamin Gaignard; Vincent Abriou; Fabien DESSENNE
> Subject: [PATCH] modetest: consider supported formats before selecting a
> DRM plane
>
> This fixes an issue where the DRM planes do not support the same pixel
> formats.
> The current implementation selects a DRM plane without checking whether
> the pixel format is supported or not. As a consequence modetest may try to
> set up a plane not supporting the user request-format, which fails.
> Modetest has to check the supported formats accross the plane list before
> selecting a candidate.
>
> Signed-off-by: Fabien Dessenne
> ---
> tests/modetest/modetest.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index
> 4761c60..866ea82 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -951,7 +951,7 @@ static int set_plane(struct device *dev, struct
> plane_arg *p)
> int crtc_x, crtc_y, crtc_w, crtc_h;
> struct crtc *crtc = NULL;
> unsigned int pipe;
> - unsigned int i;
> + unsigned int i, j;
>
> /* Find an unused plane which can be connected to our CRTC. Find
> the
>* CRTC index first, then iterate over available planes.
> @@ -974,8 +974,11 @@ static int set_plane(struct device *dev, struct
> plane_arg *p)
> if (!ovr)
> continue;
>
> - if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id)
> - plane_id = ovr->plane_id;
> + if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id) {
> + for (j = 0; j < ovr->count_formats; j++)
> + if (!strncmp(p->format_str, (char *)
> >formats[j], 4))
> + plane_id = ovr->plane_id;
> + }
> }
>
> if (!plane_id) {
> --
> 1.9.0
[PATCH] modetest: consider supported formats before selecting a DRM plane
This fixes an issue where the DRM planes do not support the same pixel
formats.
The current implementation selects a DRM plane without checking whether
the pixel format is supported or not. As a consequence modetest may try
to set up a plane not supporting the user request-format, which fails.
Modetest has to check the supported formats accross the plane list before
selecting a candidate.
Signed-off-by: Fabien Dessenne
---
tests/modetest/modetest.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 4761c60..866ea82 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -951,7 +951,7 @@ static int set_plane(struct device *dev, struct plane_arg
*p)
int crtc_x, crtc_y, crtc_w, crtc_h;
struct crtc *crtc = NULL;
unsigned int pipe;
- unsigned int i;
+ unsigned int i, j;
/* Find an unused plane which can be connected to our CRTC. Find the
* CRTC index first, then iterate over available planes.
@@ -974,8 +974,11 @@ static int set_plane(struct device *dev, struct plane_arg
*p)
if (!ovr)
continue;
- if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id)
- plane_id = ovr->plane_id;
+ if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id) {
+ for (j = 0; j < ovr->count_formats; j++)
+ if (!strncmp(p->format_str, (char *)
>formats[j], 4))
+ plane_id = ovr->plane_id;
+ }
}
if (!plane_id) {
--
1.9.0
