| Message ID | 1482149338-586-9-git-send-email-architt@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 19-12-16 om 13:08 schreef Archit Taneja: > This code has been more or less picked up from the vc4 and intel > implementations of update_plane() funcs for cursor planes. > > The update_plane() func is usually the drm_atomic_helper_update_plane > func that will issue an atomic commit with the plane updates. Such > commits are not intended to be done faster than the vsync rate. > > The legacy cursor userspace API, on the other hand, expects the kernel > to handle cursor updates immediately. > > Create a fast path in update_plane, which updates the cursor registers > and flushes the configuration. The fast path is taken when there is only > a change in the cursor's position in the crtc, or a change in the > cursor's crop co-ordinates. For anything else, we go via the slow path. > > We take the slow path even whenever the fb changes, and even when there > is currently no fb tied to the plane. This should hopefully ensure that > we always take a slow path for every new fb. The slow path will ensure > that the fb is prepared/pinned etc. > > Cc: > Signed-off-by: Archit Taneja <architt@codeaurora.org> > --- > - Don't know what to do for locking here :/ Shouldn't patch 9 be done first before 8? ~Maarten
On Mon, Dec 19, 2016 at 05:38:57PM +0530, Archit Taneja wrote: > This code has been more or less picked up from the vc4 and intel > implementations of update_plane() funcs for cursor planes. > > The update_plane() func is usually the drm_atomic_helper_update_plane > func that will issue an atomic commit with the plane updates. Such > commits are not intended to be done faster than the vsync rate. > > The legacy cursor userspace API, on the other hand, expects the kernel > to handle cursor updates immediately. > > Create a fast path in update_plane, which updates the cursor registers > and flushes the configuration. The fast path is taken when there is only > a change in the cursor's position in the crtc, or a change in the > cursor's crop co-ordinates. For anything else, we go via the slow path. > > We take the slow path even whenever the fb changes, and even when there > is currently no fb tied to the plane. This should hopefully ensure that > we always take a slow path for every new fb. The slow path will ensure > that the fb is prepared/pinned etc. > > Cc: > Signed-off-by: Archit Taneja <architt@codeaurora.org> I wonder whether it's time to extract this pattern into some helpers. I think a combo of async_check and async_flip callbacks on planes should do the trick and integrate somewhat into atomic overall. amdgpu also needs this for async flips on the primary plane. Both async_check and async_flip would take only (struct drm_plane *plane, struct drm_plane_state *state) as arguments. Maybe we should even wrap that up into an struct drm_async_flip so that we can add more stuff in the future. But all still a bit unclear to me. > --- > - Don't know what to do for locking here :/ > > drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 7 ++ > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 1 + > drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 109 +++++++++++++++++++++++++++++- > 3 files changed, 114 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > index 84fcb6e..d0c8b38 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c > @@ -778,6 +778,13 @@ void mdp5_crtc_set_pipeline(struct drm_crtc *crtc, > mdp5_ctl_set_pipeline(ctl, intf, lm); > } > > +struct mdp5_ctl *mdp5_crtc_get_ctl(struct drm_crtc *crtc) > +{ > + struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc); > + > + return mdp5_crtc->ctl; > +} > + > int mdp5_crtc_get_lm(struct drm_crtc *crtc) > { > struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc); > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > index 5be50c7..d3b728b 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h > @@ -250,6 +250,7 @@ void mdp5_plane_complete_commit(struct drm_plane *plane, > struct drm_plane *mdp5_plane_init(struct drm_device *dev, > enum drm_plane_type type); > > +struct mdp5_ctl *mdp5_crtc_get_ctl(struct drm_crtc *crtc); > uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc); > > int mdp5_crtc_get_lm(struct drm_crtc *crtc); > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > index 107bb3a..728dad7 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c > @@ -31,6 +31,14 @@ static int mdp5_plane_mode_set(struct drm_plane *plane, > struct drm_crtc *crtc, struct drm_framebuffer *fb, > struct drm_rect *src, struct drm_rect *dest); > > +static int mdp5_update_cursor_plane_legacy(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); > + > static void set_scanout_locked(struct drm_plane *plane, > struct drm_framebuffer *fb); > > @@ -246,6 +254,19 @@ static void mdp5_plane_destroy_state(struct drm_plane *plane, > .atomic_print_state = mdp5_plane_atomic_print_state, > }; > > +static const struct drm_plane_funcs mdp5_cursor_plane_funcs = { > + .update_plane = mdp5_update_cursor_plane_legacy, > + .disable_plane = drm_atomic_helper_disable_plane, > + .destroy = mdp5_plane_destroy, > + .set_property = drm_atomic_helper_plane_set_property, > + .atomic_set_property = mdp5_plane_atomic_set_property, > + .atomic_get_property = mdp5_plane_atomic_get_property, > + .reset = mdp5_plane_reset, > + .atomic_duplicate_state = mdp5_plane_duplicate_state, > + .atomic_destroy_state = mdp5_plane_destroy_state, > + .atomic_print_state = mdp5_plane_atomic_print_state, > +}; > + > static int mdp5_plane_prepare_fb(struct drm_plane *plane, > struct drm_plane_state *new_state) > { > @@ -873,6 +894,81 @@ static int mdp5_plane_mode_set(struct drm_plane *plane, > return ret; > } > > +static int mdp5_update_cursor_plane_legacy(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) > +{ > + struct drm_plane_state *plane_state, *new_plane_state; > + struct mdp5_plane_state *mdp5_pstate; > + struct drm_crtc_state *crtc_state = crtc->state; > + int ret; > + > + if (!crtc_state->active || drm_atomic_crtc_needs_modeset(crtc_state)) Until you do some kind of normal flip on the primary after a modeset no one will ever change the needs_modeset(crtc_state) if you only ever do async flips. Sounds like a bug, and vc4 doesn't check for it. Why do you need this? > + goto slow; > + > + plane_state = plane->state; > + mdp5_pstate = to_mdp5_plane_state(plane_state); > + > + /* don't use fast path if we don't have a hwpipe allocated yet */ > + if (!mdp5_pstate->hwpipe) > + goto slow; > + > + /* only allow changing of position(crtc x/y or src x/y) in fast path */ > + if (plane_state->crtc != crtc || > + plane_state->src_w != src_w || > + plane_state->src_h != src_h || > + plane_state->crtc_w != crtc_w || > + plane_state->crtc_h != crtc_h || > + !plane_state->fb || > + plane_state->fb != fb) > + goto slow; > + > + new_plane_state = mdp5_plane_duplicate_state(plane); > + if (!new_plane_state) > + return -ENOMEM; > + > + new_plane_state->src_x = src_x; > + new_plane_state->src_y = src_y; > + new_plane_state->src_w = src_w; > + new_plane_state->src_h = src_h; > + new_plane_state->crtc_x = crtc_x; > + new_plane_state->crtc_y = crtc_y; > + new_plane_state->crtc_w = crtc_w; > + new_plane_state->crtc_h = crtc_h; > + > + ret = mdp5_plane_atomic_check_with_state(crtc_state, new_plane_state); > + if (ret) > + goto slow_free; > + > + if (new_plane_state->visible) { > + struct mdp5_ctl *ctl; > + > + ret = mdp5_plane_mode_set(plane, crtc, fb, > + &new_plane_state->src, > + &new_plane_state->dst); > + WARN_ON(ret < 0); > + > + ctl = mdp5_crtc_get_ctl(crtc); > + > + mdp5_ctl_commit(ctl, mdp5_plane_get_flush(plane)); > + } > + > + *to_mdp5_plane_state(plane_state) = *to_mdp5_plane_state(new_plane_state); > + > + mdp5_plane_destroy_state(plane, new_plane_state); Even if the don't go the full monty, at least a drm_legacy_cursor_helper_update_plane which encodes the above would be good. I think it should be possible to code it up using the ->atomic_check and ->atomic_update callbacks ... But just ideas really, might be better to wait for yet another driver to explore this space before we consolidate ;-) -Daniel > + > + return 0; > +slow_free: > + mdp5_plane_destroy_state(plane, new_plane_state); > +slow: > + return drm_atomic_helper_update_plane(plane, crtc, fb, > + crtc_x, crtc_y, crtc_w, crtc_h, > + src_x, src_y, src_w, src_h); > +} > + > enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane) > { > struct mdp5_plane_state *pstate = to_mdp5_plane_state(plane->state); > @@ -920,9 +1016,16 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, enum drm_plane_type ty > mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats, > ARRAY_SIZE(mdp5_plane->formats), false); > > - ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, > - mdp5_plane->formats, mdp5_plane->nformats, > - type, NULL); > + if (type == DRM_PLANE_TYPE_CURSOR) > + ret = drm_universal_plane_init(dev, plane, 0xff, > + &mdp5_cursor_plane_funcs, > + mdp5_plane->formats, mdp5_plane->nformats, > + type, NULL); > + else > + ret = drm_universal_plane_init(dev, plane, 0xff, > + &mdp5_plane_funcs, > + mdp5_plane->formats, mdp5_plane->nformats, > + type, NULL); > if (ret) > goto fail; > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > hosted by The Linux Foundation >
On Mon, Dec 19, 2016 at 05:00:49PM +0100, Daniel Vetter wrote: > On Mon, Dec 19, 2016 at 05:38:57PM +0530, Archit Taneja wrote: > > @@ -873,6 +894,81 @@ static int mdp5_plane_mode_set(struct drm_plane *plane, > > return ret; > > } > > > > +static int mdp5_update_cursor_plane_legacy(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) > > +{ > > + struct drm_plane_state *plane_state, *new_plane_state; > > + struct mdp5_plane_state *mdp5_pstate; > > + struct drm_crtc_state *crtc_state = crtc->state; > > + int ret; > > + > > + if (!crtc_state->active || drm_atomic_crtc_needs_modeset(crtc_state)) > > Until you do some kind of normal flip on the primary after a modeset no > one will ever change the needs_modeset(crtc_state) if you only ever do > async flips. Sounds like a bug, and vc4 doesn't check for it. Why do you > need this? Self-correct: The slow-path will duplicate the crtc state also and clear needs_modeset, which means only the very first cursor move after a modeset will be slow. I think that's ok. But this kind of trickery is a good reason to try to extract this logic a bit better I think. -Daniel
On 12/19/2016 06:20 PM, Maarten Lankhorst wrote: > Op 19-12-16 om 13:08 schreef Archit Taneja: >> This code has been more or less picked up from the vc4 and intel >> implementations of update_plane() funcs for cursor planes. >> >> The update_plane() func is usually the drm_atomic_helper_update_plane >> func that will issue an atomic commit with the plane updates. Such >> commits are not intended to be done faster than the vsync rate. >> >> The legacy cursor userspace API, on the other hand, expects the kernel >> to handle cursor updates immediately. >> >> Create a fast path in update_plane, which updates the cursor registers >> and flushes the configuration. The fast path is taken when there is only >> a change in the cursor's position in the crtc, or a change in the >> cursor's crop co-ordinates. For anything else, we go via the slow path. >> >> We take the slow path even whenever the fb changes, and even when there >> is currently no fb tied to the plane. This should hopefully ensure that >> we always take a slow path for every new fb. The slow path will ensure >> that the fb is prepared/pinned etc. >> >> Cc: >> Signed-off-by: Archit Taneja <architt@codeaurora.org> >> --- >> - Don't know what to do for locking here :/ > Shouldn't patch 9 be done first before 8? Patch 9 introduces cursor drm_planes for the first time, so anything done in 8 doesn't really take effect until we add the planes in patch 9. So it's safe to have this order. Archit > > ~Maarten > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Op 20-12-16 om 07:23 schreef Archit Taneja: > > > On 12/19/2016 06:20 PM, Maarten Lankhorst wrote: >> Op 19-12-16 om 13:08 schreef Archit Taneja: >>> This code has been more or less picked up from the vc4 and intel >>> implementations of update_plane() funcs for cursor planes. >>> >>> The update_plane() func is usually the drm_atomic_helper_update_plane >>> func that will issue an atomic commit with the plane updates. Such >>> commits are not intended to be done faster than the vsync rate. >>> >>> The legacy cursor userspace API, on the other hand, expects the kernel >>> to handle cursor updates immediately. >>> >>> Create a fast path in update_plane, which updates the cursor registers >>> and flushes the configuration. The fast path is taken when there is only >>> a change in the cursor's position in the crtc, or a change in the >>> cursor's crop co-ordinates. For anything else, we go via the slow path. >>> >>> We take the slow path even whenever the fb changes, and even when there >>> is currently no fb tied to the plane. This should hopefully ensure that >>> we always take a slow path for every new fb. The slow path will ensure >>> that the fb is prepared/pinned etc. >>> >>> Cc: >>> Signed-off-by: Archit Taneja <architt@codeaurora.org> >>> --- >>> - Don't know what to do for locking here :/ >> Shouldn't patch 9 be done first before 8? > > Patch 9 introduces cursor drm_planes for the first time, so > anything done in 8 doesn't really take effect until we add > the planes in patch 9. So it's safe to have this order. The other way around will allow you to test with the normal atomic helpers first, and then if the hack breaks you're able to bisect it correctly and revert it, that's why I think it should be the other way around. :) ~Maarten
On 12/20/2016 07:10 PM, Maarten Lankhorst wrote: > Op 20-12-16 om 07:23 schreef Archit Taneja: >> >> >> On 12/19/2016 06:20 PM, Maarten Lankhorst wrote: >>> Op 19-12-16 om 13:08 schreef Archit Taneja: >>>> This code has been more or less picked up from the vc4 and intel >>>> implementations of update_plane() funcs for cursor planes. >>>> >>>> The update_plane() func is usually the drm_atomic_helper_update_plane >>>> func that will issue an atomic commit with the plane updates. Such >>>> commits are not intended to be done faster than the vsync rate. >>>> >>>> The legacy cursor userspace API, on the other hand, expects the kernel >>>> to handle cursor updates immediately. >>>> >>>> Create a fast path in update_plane, which updates the cursor registers >>>> and flushes the configuration. The fast path is taken when there is only >>>> a change in the cursor's position in the crtc, or a change in the >>>> cursor's crop co-ordinates. For anything else, we go via the slow path. >>>> >>>> We take the slow path even whenever the fb changes, and even when there >>>> is currently no fb tied to the plane. This should hopefully ensure that >>>> we always take a slow path for every new fb. The slow path will ensure >>>> that the fb is prepared/pinned etc. >>>> >>>> Cc: >>>> Signed-off-by: Archit Taneja <architt@codeaurora.org> >>>> --- >>>> - Don't know what to do for locking here :/ >>> Shouldn't patch 9 be done first before 8? >> >> Patch 9 introduces cursor drm_planes for the first time, so >> anything done in 8 doesn't really take effect until we add >> the planes in patch 9. So it's safe to have this order. > The other way around will allow you to test with the normal atomic helpers first, and then if the hack breaks you're able to bisect it correctly and revert it, that's why I think it should be the other way around. :) Yeah, makes sense when you put it that way :). Will make sure the hack commit comes after the cursor planes are supported the regular atomic way. Archit > > ~Maarten >
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c index 84fcb6e..d0c8b38 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c @@ -778,6 +778,13 @@ void mdp5_crtc_set_pipeline(struct drm_crtc *crtc, mdp5_ctl_set_pipeline(ctl, intf, lm); } +struct mdp5_ctl *mdp5_crtc_get_ctl(struct drm_crtc *crtc) +{ + struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc); + + return mdp5_crtc->ctl; +} + int mdp5_crtc_get_lm(struct drm_crtc *crtc) { struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc); diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h index 5be50c7..d3b728b 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h @@ -250,6 +250,7 @@ void mdp5_plane_complete_commit(struct drm_plane *plane, struct drm_plane *mdp5_plane_init(struct drm_device *dev, enum drm_plane_type type); +struct mdp5_ctl *mdp5_crtc_get_ctl(struct drm_crtc *crtc); uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc); int mdp5_crtc_get_lm(struct drm_crtc *crtc); diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index 107bb3a..728dad7 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c @@ -31,6 +31,14 @@ static int mdp5_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_rect *src, struct drm_rect *dest); +static int mdp5_update_cursor_plane_legacy(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); + static void set_scanout_locked(struct drm_plane *plane, struct drm_framebuffer *fb); @@ -246,6 +254,19 @@ static void mdp5_plane_destroy_state(struct drm_plane *plane, .atomic_print_state = mdp5_plane_atomic_print_state, }; +static const struct drm_plane_funcs mdp5_cursor_plane_funcs = { + .update_plane = mdp5_update_cursor_plane_legacy, + .disable_plane = drm_atomic_helper_disable_plane, + .destroy = mdp5_plane_destroy, + .set_property = drm_atomic_helper_plane_set_property, + .atomic_set_property = mdp5_plane_atomic_set_property, + .atomic_get_property = mdp5_plane_atomic_get_property, + .reset = mdp5_plane_reset, + .atomic_duplicate_state = mdp5_plane_duplicate_state, + .atomic_destroy_state = mdp5_plane_destroy_state, + .atomic_print_state = mdp5_plane_atomic_print_state, +}; + static int mdp5_plane_prepare_fb(struct drm_plane *plane, struct drm_plane_state *new_state) { @@ -873,6 +894,81 @@ static int mdp5_plane_mode_set(struct drm_plane *plane, return ret; } +static int mdp5_update_cursor_plane_legacy(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) +{ + struct drm_plane_state *plane_state, *new_plane_state; + struct mdp5_plane_state *mdp5_pstate; + struct drm_crtc_state *crtc_state = crtc->state; + int ret; + + if (!crtc_state->active || drm_atomic_crtc_needs_modeset(crtc_state)) + goto slow; + + plane_state = plane->state; + mdp5_pstate = to_mdp5_plane_state(plane_state); + + /* don't use fast path if we don't have a hwpipe allocated yet */ + if (!mdp5_pstate->hwpipe) + goto slow; + + /* only allow changing of position(crtc x/y or src x/y) in fast path */ + if (plane_state->crtc != crtc || + plane_state->src_w != src_w || + plane_state->src_h != src_h || + plane_state->crtc_w != crtc_w || + plane_state->crtc_h != crtc_h || + !plane_state->fb || + plane_state->fb != fb) + goto slow; + + new_plane_state = mdp5_plane_duplicate_state(plane); + if (!new_plane_state) + return -ENOMEM; + + new_plane_state->src_x = src_x; + new_plane_state->src_y = src_y; + new_plane_state->src_w = src_w; + new_plane_state->src_h = src_h; + new_plane_state->crtc_x = crtc_x; + new_plane_state->crtc_y = crtc_y; + new_plane_state->crtc_w = crtc_w; + new_plane_state->crtc_h = crtc_h; + + ret = mdp5_plane_atomic_check_with_state(crtc_state, new_plane_state); + if (ret) + goto slow_free; + + if (new_plane_state->visible) { + struct mdp5_ctl *ctl; + + ret = mdp5_plane_mode_set(plane, crtc, fb, + &new_plane_state->src, + &new_plane_state->dst); + WARN_ON(ret < 0); + + ctl = mdp5_crtc_get_ctl(crtc); + + mdp5_ctl_commit(ctl, mdp5_plane_get_flush(plane)); + } + + *to_mdp5_plane_state(plane_state) = *to_mdp5_plane_state(new_plane_state); + + mdp5_plane_destroy_state(plane, new_plane_state); + + return 0; +slow_free: + mdp5_plane_destroy_state(plane, new_plane_state); +slow: + return drm_atomic_helper_update_plane(plane, crtc, fb, + crtc_x, crtc_y, crtc_w, crtc_h, + src_x, src_y, src_w, src_h); +} + enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane) { struct mdp5_plane_state *pstate = to_mdp5_plane_state(plane->state); @@ -920,9 +1016,16 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, enum drm_plane_type ty mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats, ARRAY_SIZE(mdp5_plane->formats), false); - ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, - mdp5_plane->formats, mdp5_plane->nformats, - type, NULL); + if (type == DRM_PLANE_TYPE_CURSOR) + ret = drm_universal_plane_init(dev, plane, 0xff, + &mdp5_cursor_plane_funcs, + mdp5_plane->formats, mdp5_plane->nformats, + type, NULL); + else + ret = drm_universal_plane_init(dev, plane, 0xff, + &mdp5_plane_funcs, + mdp5_plane->formats, mdp5_plane->nformats, + type, NULL); if (ret) goto fail;
This code has been more or less picked up from the vc4 and intel implementations of update_plane() funcs for cursor planes. The update_plane() func is usually the drm_atomic_helper_update_plane func that will issue an atomic commit with the plane updates. Such commits are not intended to be done faster than the vsync rate. The legacy cursor userspace API, on the other hand, expects the kernel to handle cursor updates immediately. Create a fast path in update_plane, which updates the cursor registers and flushes the configuration. The fast path is taken when there is only a change in the cursor's position in the crtc, or a change in the cursor's crop co-ordinates. For anything else, we go via the slow path. We take the slow path even whenever the fb changes, and even when there is currently no fb tied to the plane. This should hopefully ensure that we always take a slow path for every new fb. The slow path will ensure that the fb is prepared/pinned etc. Cc: Signed-off-by: Archit Taneja <architt@codeaurora.org> --- - Don't know what to do for locking here :/ drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 7 ++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 1 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 109 +++++++++++++++++++++++++++++- 3 files changed, 114 insertions(+), 3 deletions(-)