Hi Daniel,
thank you for the comments.
Am Dienstag, den 22.09.2015, 11:38 +0200 schrieb Daniel Vetter: [...]
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c new file mode 100644 index 0000000..fc071fe --- /dev/null +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -0,0 +1,471 @@
[...]
+static int mtk_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state,bool async)+{
- return drm_atomic_helper_commit(dev, state, false);
This isn't a proper async commit operation, it will still block userspace unecessarily. See e.g. the vc4 patches for a proper one.
I'll drop this function and assign .atomic_commit to drm_atomic_helper_commit directly.
[...]
+static int mtk_plane_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)+{
[...]
- ret = drm_plane_helper_check_update(plane, state->crtc, fb,
&src, &dest, &clip,DRM_PLANE_HELPER_NO_SCALING,DRM_PLANE_HELPER_NO_SCALING,true, true, &visible);- if (ret)
return ret;- if (!visible)
return 0;- mtk_plane->disp_size = (dest.y2 - dest.y1) << 16 | (dest.x2 - dest.x1);
I think it might work out ok but it's very fragile to update object state from your atomic_check hooks - atomic allows a TEST_ONLY mode and if that's used (generic userspace will do that a few times for each frame at least) then you clobber shared state. Instead it's better to store that in your own mtk_plane_state which subclasses drm_plane_state.
Ok, will do.
[...]
+static void mtk_plane_attach_zpos_property(struct drm_plane *plane,
unsigned int zpos, unsigned int max_plane)+{
- struct drm_device *dev = plane->dev;
- struct mtk_drm_private *dev_priv = dev->dev_private;
- struct drm_property *prop;
- prop = dev_priv->plane_zpos_property;
- if (!prop) {
prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE,"zpos", 0, max_plane - 1);I know that there's lots of other drivers exposing zpos already, but I really think we should standardize this properly and document what it means. So
- add a bit more text to the kerneldoc/doobook, especially what should happen when there's a conflict in zpos.
- move zpos registration/decoding into drm core, which means adding it to drm_plane_state
- have an open-source implementation using this somewhere (ddx, wayland, hwc, ...).
I think for now it's better to drop the zpos property from initial mediatek enabling.
Alright, I'll separate this from the initial patchset.
best regards Philipp