On 7/14/21 3:30 AM, Thomas Zimmermann wrote:
@@ -301,8 +301,12 @@ static void vmw_print_capabilities2(uint32_t capabilities2)
DRM_INFO(" Grow oTable.\n");
These macros have been out of fashion for a while. There's drm_info(), drm_warn(), drm_err(), etc as replacements. They also print device information. Applis here and for the rest of the patchset.
Yea, that's messy, I'll go ahead and cleanup some of our logging in v2.
+ BUG_ON((*bo)->resource->mem_type != VMW_PL_MOB);
BUG_ON() crashes the kernel. The prefered way is to use drm_WARN_*() and return.
This one is really an assert. This should never happen, I'm not sure if it's worth even testing for this because the driver is broken if that happens.
That's something like page-flipping with alternating BO's and shadow buffering?
You really want a cursor plane to hold this information.
I briefly looked through vmwgfx and it has all these fail-able code in its atomic-update path. The patches here only make things worse. With cursor planes, you can do all the preparation in atomic_check and prepare_fb, and store the
intermediate state/mappings/etc in the plane state.
The ast driver started with a design like this one here and then we moved it to cursor planes. Ast has now none of the mentioned problems. Relevant code is at [1][2].
Yea, I was hoping to the the cursor mob's first and then go back and refactor the error paths for all cursor modes. I do agree that it's not the cleanest approach because it leaves us in a bit broken state until the refactor lands. I'll take out the cursor mob change from v2 of this patchset to give Martin some time to clean up the patch a bit. I'll probably send that out as a separate "cursor mob and atomic errors refactor/fixes" patchset later.
Thanks for the review!
z
