| Message ID | 20231108163647.106853-18-harry.wentland@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Color Pipeline API w/ VKMS | expand |
On Wed, 8 Nov 2023 11:36:36 -0500 Harry Wentland <harry.wentland@amd.com> wrote: > With the introduction of the pre-blending color pipeline we > can no longer have color operations that don't have a clear > position in the color pipeline. We deprecate all existing > plane properties. For upstream drivers those are: > - COLOR_ENCODING > - COLOR_RANGE > > Userspace that registers with the > DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE will be permitted to set > the COLOR_PIPELINE plane property and drm_colorop properties. > But it won't be allowed to set COLOR_ENCODING and > COLOR_RANGE. Userspace that does not set this client cap > will not be permitted to touch the color pipeline. > > Signed-off-by: Harry Wentland <harry.wentland@amd.com> > --- > drivers/gpu/drm/drm_atomic_uapi.c | 23 ++++++++++++++++++++++- > drivers/gpu/drm/drm_ioctl.c | 7 +++++++ > include/drm/drm_file.h | 7 +++++++ > include/uapi/drm/drm.h | 15 +++++++++++++++ > 4 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c > index c6629fdaa114..69c56982e2d0 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -610,10 +610,26 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, > } else if (property == plane->zpos_property) { > state->zpos = val; > } else if (property == plane->color_encoding_property) { > + if (file_priv->plane_color_pipeline) { > + drm_dbg_atomic(dev, > + "Setting COLOR_PIPELINE plane property not permitted when DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set\n"); > + return -EINVAL; > + } > state->color_encoding = val; > } else if (property == plane->color_range_property) { > + if (file_priv->plane_color_pipeline) { > + drm_dbg_atomic(dev, > + "Setting COLOR_PIPELINE plane property not permitted when DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set\n"); > + return -EINVAL; > + } > state->color_range = val; The above two cases mention COLOR_PIPELINE when they should probably refer to something else. Thanks, pq > } else if (property == plane->color_pipeline_property) { > + if (!file_priv->plane_color_pipeline) { > + drm_dbg_atomic(dev, > + "Setting COLOR_PIPELINE plane property not permitted unless DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set\n"); > + return -EINVAL; > + } > + > /* find DRM colorop object */ > struct drm_colorop *colorop = NULL; > colorop = drm_colorop_find(dev, file_priv, val); > @@ -1158,6 +1174,12 @@ int drm_atomic_set_property(struct drm_atomic_state *state, > break; > } > case DRM_MODE_OBJECT_COLOROP: { > + if (!file_priv->plane_color_pipeline) { > + drm_dbg_atomic(prop->dev, > + "[OBJECT:%d] is a colorop but DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE not set\n", > + obj->id); > + ret = -EINVAL; > + } > struct drm_colorop *colorop = obj_to_colorop(obj); > struct drm_colorop_state *colorop_state; > > @@ -1170,7 +1192,6 @@ int drm_atomic_set_property(struct drm_atomic_state *state, > ret = drm_atomic_colorop_set_property(colorop, > colorop_state, file_priv, > prop, prop_value); > - > break; > } > default: > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 44fda68c28ae..0d869658e13e 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -361,6 +361,13 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) > return -EINVAL; > file_priv->writeback_connectors = req->value; > break; > + case DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE: > + if (!file_priv->atomic) > + return -EINVAL; > + if (req->value > 1) > + return -EINVAL; > + file_priv->plane_color_pipeline = req->value; > + break; > default: > return -EINVAL; > } > diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h > index e1b5b4282f75..bf11b646c898 100644 > --- a/include/drm/drm_file.h > +++ b/include/drm/drm_file.h > @@ -204,6 +204,13 @@ struct drm_file { > */ > bool writeback_connectors; > > + /** > + * @plane_color_pipeline: > + * > + * True if client understands plane color pipelines > + */ > + bool plane_color_pipeline; > + > /** > * @was_master: > * > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > index 841d393fb84e..2576b170e8d0 100644 > --- a/include/uapi/drm/drm.h > +++ b/include/uapi/drm/drm.h > @@ -842,6 +842,21 @@ struct drm_get_cap { > */ > #define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS 5 > > +/** > + * DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE > + * > + * If set to 1, the DRM core will > + * - expose plane COLOR_PIPELINE properties for pre-blending color management. > + * - reject setting of these plane properties: > + * - COLOR_ENCODING > + * - COLOR_RANGE > + * > + * The client must enable &DRM_CLIENT_CAP_ATOMIC first. > + * > + * This capability is currently in development. > + */ > +#define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE 7 > + > /* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */ > struct drm_set_client_cap { > __u64 capability;
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index c6629fdaa114..69c56982e2d0 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -610,10 +610,26 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane, } else if (property == plane->zpos_property) { state->zpos = val; } else if (property == plane->color_encoding_property) { + if (file_priv->plane_color_pipeline) { + drm_dbg_atomic(dev, + "Setting COLOR_PIPELINE plane property not permitted when DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set\n"); + return -EINVAL; + } state->color_encoding = val; } else if (property == plane->color_range_property) { + if (file_priv->plane_color_pipeline) { + drm_dbg_atomic(dev, + "Setting COLOR_PIPELINE plane property not permitted when DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set\n"); + return -EINVAL; + } state->color_range = val; } else if (property == plane->color_pipeline_property) { + if (!file_priv->plane_color_pipeline) { + drm_dbg_atomic(dev, + "Setting COLOR_PIPELINE plane property not permitted unless DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set\n"); + return -EINVAL; + } + /* find DRM colorop object */ struct drm_colorop *colorop = NULL; colorop = drm_colorop_find(dev, file_priv, val); @@ -1158,6 +1174,12 @@ int drm_atomic_set_property(struct drm_atomic_state *state, break; } case DRM_MODE_OBJECT_COLOROP: { + if (!file_priv->plane_color_pipeline) { + drm_dbg_atomic(prop->dev, + "[OBJECT:%d] is a colorop but DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE not set\n", + obj->id); + ret = -EINVAL; + } struct drm_colorop *colorop = obj_to_colorop(obj); struct drm_colorop_state *colorop_state; @@ -1170,7 +1192,6 @@ int drm_atomic_set_property(struct drm_atomic_state *state, ret = drm_atomic_colorop_set_property(colorop, colorop_state, file_priv, prop, prop_value); - break; } default: diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 44fda68c28ae..0d869658e13e 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -361,6 +361,13 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) return -EINVAL; file_priv->writeback_connectors = req->value; break; + case DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE: + if (!file_priv->atomic) + return -EINVAL; + if (req->value > 1) + return -EINVAL; + file_priv->plane_color_pipeline = req->value; + break; default: return -EINVAL; } diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index e1b5b4282f75..bf11b646c898 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -204,6 +204,13 @@ struct drm_file { */ bool writeback_connectors; + /** + * @plane_color_pipeline: + * + * True if client understands plane color pipelines + */ + bool plane_color_pipeline; + /** * @was_master: * diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 841d393fb84e..2576b170e8d0 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -842,6 +842,21 @@ struct drm_get_cap { */ #define DRM_CLIENT_CAP_WRITEBACK_CONNECTORS 5 +/** + * DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE + * + * If set to 1, the DRM core will + * - expose plane COLOR_PIPELINE properties for pre-blending color management. + * - reject setting of these plane properties: + * - COLOR_ENCODING + * - COLOR_RANGE + * + * The client must enable &DRM_CLIENT_CAP_ATOMIC first. + * + * This capability is currently in development. + */ +#define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE 7 + /* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */ struct drm_set_client_cap { __u64 capability;
With the introduction of the pre-blending color pipeline we can no longer have color operations that don't have a clear position in the color pipeline. We deprecate all existing plane properties. For upstream drivers those are: - COLOR_ENCODING - COLOR_RANGE Userspace that registers with the DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE will be permitted to set the COLOR_PIPELINE plane property and drm_colorop properties. But it won't be allowed to set COLOR_ENCODING and COLOR_RANGE. Userspace that does not set this client cap will not be permitted to touch the color pipeline. Signed-off-by: Harry Wentland <harry.wentland@amd.com> --- drivers/gpu/drm/drm_atomic_uapi.c | 23 ++++++++++++++++++++++- drivers/gpu/drm/drm_ioctl.c | 7 +++++++ include/drm/drm_file.h | 7 +++++++ include/uapi/drm/drm.h | 15 +++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-)