| Message ID | 20181004202446.22905-6-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | atomic helper cleanup, take 2 | expand |
Hi! I've sent out patches that replace 05/21 and 07/21. Since I'm on travel, I'm not sure I'll be able to incorporate them in a pull before the merge window though. /Thomas On 10/04/2018 10:24 PM, Daniel Vetter wrote: ...
On Thu, Oct 04, 2018 at 10:40:16PM +0000, Thomas Hellstrom wrote: > Hi! > > I've sent out patches that replace 05/21 and 07/21. Since I'm on travel, > I'm not sure I'll be able to incorporate them in a pull before the merge > window though. is_implicit is taken care of with those indeed. But patch 07 seems still needed, at least I'm not seeing where you remove the custom page_flip checks. -Daniel
On Fri, Oct 05, 2018 at 08:43:51AM +0200, Daniel Vetter wrote: > On Thu, Oct 04, 2018 at 10:40:16PM +0000, Thomas Hellstrom wrote: > > Hi! > > > > I've sent out patches that replace 05/21 and 07/21. Since I'm on travel, > > I'm not sure I'll be able to incorporate them in a pull before the merge > > window though. > > is_implicit is taken care of with those indeed. But patch 07 seems still > needed, at least I'm not seeing where you remove the custom page_flip > checks. Clarification, now that coffee kicks in: I wrote the FIXME comments since an atomic driver should not have _any_ checks in its page_flip callback beyond what the helper does. Only thing you can do in there is fix up uapi inconsistency, because your driver-specific does something funky. Like in your set_config callback, where you clear a parameter that old versions of vmwgfx userspace don't clear. So end result for addressing patch 07 is that you directly put the helper function into the vtable. You removed a bunch of checks, but there's still a bit left. For why this matters: I'd like to, at least long term, move the legacy2atomic helpers into the core and just enforce them as the default. That's why I need to know which drivers actually need to patch up things, and which just have code in there that should be moved to atomic_check. -Daniel
On 10/05/2018 09:48 AM, Daniel Vetter wrote: > On Fri, Oct 05, 2018 at 08:43:51AM +0200, Daniel Vetter wrote: >> On Thu, Oct 04, 2018 at 10:40:16PM +0000, Thomas Hellstrom wrote: >>> Hi! >>> >>> I've sent out patches that replace 05/21 and 07/21. Since I'm on travel, >>> I'm not sure I'll be able to incorporate them in a pull before the merge >>> window though. >> is_implicit is taken care of with those indeed. But patch 07 seems still >> needed, at least I'm not seeing where you remove the custom page_flip >> checks. > Clarification, now that coffee kicks in: I wrote the FIXME comments since > an atomic driver should not have _any_ checks in its page_flip callback > beyond what the helper does. Only thing you can do in there is fix up uapi > inconsistency, because your driver-specific does something funky. Like in > your set_config callback, where you clear a parameter that old versions of > vmwgfx userspace don't clear. > > So end result for addressing patch 07 is that you directly put the helper > function into the vtable. You removed a bunch of checks, but there's still > a bit left. Ah, are you referring to the stdu->defined check? I think that's just a leftover sanity check that I can remove. Or were you referring to something else? /Thomas
On Fri, Oct 5, 2018 at 8:19 PM Thomas Hellstrom <thellstrom@vmware.com> wrote: > > On 10/05/2018 09:48 AM, Daniel Vetter wrote: > > On Fri, Oct 05, 2018 at 08:43:51AM +0200, Daniel Vetter wrote: > >> On Thu, Oct 04, 2018 at 10:40:16PM +0000, Thomas Hellstrom wrote: > >>> Hi! > >>> > >>> I've sent out patches that replace 05/21 and 07/21. Since I'm on travel, > >>> I'm not sure I'll be able to incorporate them in a pull before the merge > >>> window though. > >> is_implicit is taken care of with those indeed. But patch 07 seems still > >> needed, at least I'm not seeing where you remove the custom page_flip > >> checks. > > Clarification, now that coffee kicks in: I wrote the FIXME comments since > > an atomic driver should not have _any_ checks in its page_flip callback > > beyond what the helper does. Only thing you can do in there is fix up uapi > > inconsistency, because your driver-specific does something funky. Like in > > your set_config callback, where you clear a parameter that old versions of > > vmwgfx userspace don't clear. > > > > So end result for addressing patch 07 is that you directly put the helper > > function into the vtable. You removed a bunch of checks, but there's still > > a bit left. > > Ah, are you referring to the stdu->defined check? I think that's just a > leftover sanity check that I can remove. Or were you referring to > something else? Yeah I think that's the last one. I didn't look what that could be doing. -Daniel
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 91ad45b22fee..a50fb0360317 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -2303,12 +2303,6 @@ vmw_du_connector_atomic_set_property(struct drm_connector *connector, if (property == dev_priv->implicit_placement_property) { vcs->is_implicit = val; - - /* - * We should really be doing a drm_atomic_commit() to - * commit the new state, but since this doesn't cause - * an immedate state change, this is probably ok - */ du->is_implicit = vcs->is_implicit; } else { return -EINVAL;
The core _does_ the call to drm_atomic_commit for you. That's pretty much the entire point of having the fancy new atomic_set/get_prop callbacks. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> Cc: Sinclair Yeh <syeh@vmware.com> Cc: Thomas Hellstrom <thellstrom@vmware.com> --- drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 6 ------ 1 file changed, 6 deletions(-)