| Message ID | 799e17732ff6218ba6ea2287397faf24b7660918.1485510281.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jyri, Thank you for the patch. On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote: > Move drm_atomic_helper_commit_modeset_enables() call to before > drm_atomic_helper_commit_planes() call and have a > omap_atomic_wait_for_completion() call after both. > > With the current dss dispc implementation we have to enable the new > modeset before we can commit planes. The dispc ovl configuration > relies on the video mode configuration been written into the HW when > the ovl configuration is calculated. > > This approach is not ideal because after a mode change the plane > update is executed only after the first vblank interrupt. The dispc > implementation should be fixed so that it is able use uncommitted drm > state information. information. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct > omap_atomic_state_commit *commit) dispc_runtime_get(); > > drm_atomic_helper_commit_modeset_disables(dev, old_state); > - drm_atomic_helper_commit_planes(dev, old_state, 0); > + > + /* With the current dss dispc implementation we have to enable > + * the new modeset before we can commit planes. The dispc ovl > + * configuration relies on the video mode configuration been > + * written into the HW when the ovl configuration is > + * calculated. > + * > + * This approach is not ideal because after a mode change the > + * plane update is executed only after the first vblank > + * interrupt. The dispc implementation should be fixed so that > + * it is able use uncommitted drm state information. Any chance you could fix the dispc implementation ? ;-) Otherwise, could you rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and zpos" ? > + */ > drm_atomic_helper_commit_modeset_enables(dev, old_state); > + omap_atomic_wait_for_completion(dev, old_state); > + > + drm_atomic_helper_commit_planes(dev, old_state, 0); > > omap_atomic_wait_for_completion(dev, old_state);
On 28/01/17 18:17, Laurent Pinchart wrote: > Hi Jyri, > > Thank you for the patch. > > On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote: >> Move drm_atomic_helper_commit_modeset_enables() call to before >> drm_atomic_helper_commit_planes() call and have a >> omap_atomic_wait_for_completion() call after both. >> >> With the current dss dispc implementation we have to enable the new >> modeset before we can commit planes. The dispc ovl configuration >> relies on the video mode configuration been written into the HW when >> the ovl configuration is calculated. >> >> This approach is not ideal because after a mode change the plane >> update is executed only after the first vblank interrupt. The dispc >> implementation should be fixed so that it is able use uncommitted drm >> state information. information. >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> --- >> drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++- >> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct >> omap_atomic_state_commit *commit) dispc_runtime_get(); >> >> drm_atomic_helper_commit_modeset_disables(dev, old_state); >> - drm_atomic_helper_commit_planes(dev, old_state, 0); >> + >> + /* With the current dss dispc implementation we have to enable >> + * the new modeset before we can commit planes. The dispc ovl >> + * configuration relies on the video mode configuration been >> + * written into the HW when the ovl configuration is >> + * calculated. >> + * >> + * This approach is not ideal because after a mode change the >> + * plane update is executed only after the first vblank >> + * interrupt. The dispc implementation should be fixed so that >> + * it is able use uncommitted drm state information. > > Any chance you could fix the dispc implementation ? ;-) Otherwise, could you Dispc change is on our todo, but the size of the change won't fit into "fix" definition =). There's a lot of state that the dispc side is looking at, which at the moment doesn't come from drm side. Tomi
On 01/28/17 18:17, Laurent Pinchart wrote: > Hi Jyri, > > Thank you for the patch. > > On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote: >> Move drm_atomic_helper_commit_modeset_enables() call to before >> drm_atomic_helper_commit_planes() call and have a >> omap_atomic_wait_for_completion() call after both. >> >> With the current dss dispc implementation we have to enable the new >> modeset before we can commit planes. The dispc ovl configuration >> relies on the video mode configuration been written into the HW when >> the ovl configuration is calculated. >> >> This approach is not ideal because after a mode change the plane >> update is executed only after the first vblank interrupt. The dispc >> implementation should be fixed so that it is able use uncommitted drm >> state information. information. >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> --- >> drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++- >> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct >> omap_atomic_state_commit *commit) dispc_runtime_get(); >> >> drm_atomic_helper_commit_modeset_disables(dev, old_state); >> - drm_atomic_helper_commit_planes(dev, old_state, 0); >> + >> + /* With the current dss dispc implementation we have to enable >> + * the new modeset before we can commit planes. The dispc ovl >> + * configuration relies on the video mode configuration been >> + * written into the HW when the ovl configuration is >> + * calculated. >> + * >> + * This approach is not ideal because after a mode change the >> + * plane update is executed only after the first vblank >> + * interrupt. The dispc implementation should be fixed so that >> + * it is able use uncommitted drm state information. > Any chance you could fix the dispc implementation ? ;-) Otherwise, could you I am currently studying how to do it, but it tricky... really tricky. In DRM terms, the plane configuration is affected by the connector where the crtc is connected to. The whole DSS should have to made understand drm state structures. There certainly wont be any "fix" for 4.10, probably nothing for 4.11 either. > rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and > zpos" ? > Thanks, I'll do that. BR, Jyri
Hi Jyri, On Monday 30 Jan 2017 13:11:56 Jyri Sarha wrote: > On 01/28/17 18:17, Laurent Pinchart wrote: > > On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote: > >> Move drm_atomic_helper_commit_modeset_enables() call to before > >> drm_atomic_helper_commit_planes() call and have a > >> omap_atomic_wait_for_completion() call after both. > >> > >> With the current dss dispc implementation we have to enable the new > >> modeset before we can commit planes. The dispc ovl configuration > >> relies on the video mode configuration been written into the HW when > >> the ovl configuration is calculated. > >> > >> This approach is not ideal because after a mode change the plane > >> update is executed only after the first vblank interrupt. The dispc > >> implementation should be fixed so that it is able use uncommitted drm > >> state information. information. > >> > >> Signed-off-by: Jyri Sarha <jsarha@ti.com> > >> --- > >> > >> drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++- > >> 1 file changed, 15 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644 > >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c > >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > >> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct > >> omap_atomic_state_commit *commit) dispc_runtime_get(); > >> > >> drm_atomic_helper_commit_modeset_disables(dev, old_state); > >> > >> - drm_atomic_helper_commit_planes(dev, old_state, 0); > >> + > >> + /* With the current dss dispc implementation we have to enable > >> + * the new modeset before we can commit planes. The dispc ovl > >> + * configuration relies on the video mode configuration been > >> + * written into the HW when the ovl configuration is > >> + * calculated. > >> + * > >> + * This approach is not ideal because after a mode change the > >> + * plane update is executed only after the first vblank > >> + * interrupt. The dispc implementation should be fixed so that > >> + * it is able use uncommitted drm state information. > > > > Any chance you could fix the dispc implementation ? ;-) Otherwise, could > > you > > I am currently studying how to do it, but it tricky... really tricky. In > DRM terms, the plane configuration is affected by the connector where > the crtc is connected to. The whole DSS should have to made understand > drm state structures. There certainly wont be any "fix" for 4.10, > probably nothing for 4.11 either. > > > rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and > > zpos" ? > > Thanks, I'll do that. If you intend on merging this patch as a v4.10 fix then there's no need to rebase. If it targets v4.11, the above-mentioned series will likely go in first.
On 01/30/17 13:15, Laurent Pinchart wrote: >>> rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and >>> zpos" ? >> Thanks, I'll do that. > If you intend on merging this patch as a v4.10 fix then there's no need to > rebase. If it targets v4.11, the above-mentioned series will likely go in > first. Well perhaps this patch should go to v4.10 already. Because of the bug a plane commit may fail if the screen is blanked and there is no pclk available at the time the plane HW is configured. Tomi, what do you think? BR, Jyri
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct omap_atomic_state_commit *commit) dispc_runtime_get(); drm_atomic_helper_commit_modeset_disables(dev, old_state); - drm_atomic_helper_commit_planes(dev, old_state, 0); + + /* With the current dss dispc implementation we have to enable + * the new modeset before we can commit planes. The dispc ovl + * configuration relies on the video mode configuration been + * written into the HW when the ovl configuration is + * calculated. + * + * This approach is not ideal because after a mode change the + * plane update is executed only after the first vblank + * interrupt. The dispc implementation should be fixed so that + * it is able use uncommitted drm state information. + */ drm_atomic_helper_commit_modeset_enables(dev, old_state); + omap_atomic_wait_for_completion(dev, old_state); + + drm_atomic_helper_commit_planes(dev, old_state, 0); omap_atomic_wait_for_completion(dev, old_state);
Move drm_atomic_helper_commit_modeset_enables() call to before drm_atomic_helper_commit_planes() call and have a omap_atomic_wait_for_completion() call after both. With the current dss dispc implementation we have to enable the new modeset before we can commit planes. The dispc ovl configuration relies on the video mode configuration been written into the HW when the ovl configuration is calculated. This approach is not ideal because after a mode change the plane update is executed only after the first vblank interrupt. The dispc implementation should be fixed so that it is able use uncommitted drm state information. information. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)