diff mbox

[RFCv1,03/12] drm: add DRM_MODE_PROP_DYNAMIC property flag

Message ID 1381020350-1125-4-git-send-email-robdclark@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rob Clark Oct. 6, 2013, 12:45 a.m. UTC
This indicates to userspace that the property is something that can
be set dynamically without requiring a "test" step to check if the
hw is capable.  This allows a userspace compositor, such as weston,
to avoid an extra ioctl to check whether it needs to fall-back to
GPU to composite some surface prior to submission of GPU render
commands.
---
 include/uapi/drm/drm_mode.h | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Ville Syrjälä Oct. 7, 2013, 1:46 p.m. UTC | #1
On Sat, Oct 05, 2013 at 08:45:41PM -0400, Rob Clark wrote:
> This indicates to userspace that the property is something that can
> be set dynamically without requiring a "test" step to check if the
> hw is capable.  This allows a userspace compositor, such as weston,
> to avoid an extra ioctl to check whether it needs to fall-back to
> GPU to composite some surface prior to submission of GPU render
> commands.
> ---
>  include/uapi/drm/drm_mode.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 35921ba..15db837 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -232,6 +232,15 @@ struct drm_mode_get_connector {
>  #define DRM_MODE_PROP_BLOB	(1<<4)
>  #define DRM_MODE_PROP_BITMASK	(1<<5) /* bitmask of enumerated types */
>  #define DRM_MODE_PROP_OBJECT	(1<<6) /* drm mode object */
> +/* Properties that are not dynamic cannot safely be changed without a
> + * atomic-modeset / atomic-pageflip test step.  But if userspace is
> + * only changing dynamic properties, it is guaranteed that the change
> + * will not exceed hw limits, so no test step is required.
> + *
> + * Note that fb_id properties are a bit ambiguous.. they of course can
> + * be changed dynamically, assuming the pixel format does not change.
> + */
> +#define DRM_MODE_PROP_DYNAMIC	(1<<24)

I'm still not convinced that this is useful. We can't even flag the FB
ID with this flag since the FB encompasses stride/bpp/etc. parameters
that for sure can't just be changed w/o first checking the new
configuration.
Rob Clark Oct. 7, 2013, 2:46 p.m. UTC | #2
On Mon, Oct 7, 2013 at 9:46 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Sat, Oct 05, 2013 at 08:45:41PM -0400, Rob Clark wrote:
>> This indicates to userspace that the property is something that can
>> be set dynamically without requiring a "test" step to check if the
>> hw is capable.  This allows a userspace compositor, such as weston,
>> to avoid an extra ioctl to check whether it needs to fall-back to
>> GPU to composite some surface prior to submission of GPU render
>> commands.
>> ---
>>  include/uapi/drm/drm_mode.h | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>> index 35921ba..15db837 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -232,6 +232,15 @@ struct drm_mode_get_connector {
>>  #define DRM_MODE_PROP_BLOB   (1<<4)
>>  #define DRM_MODE_PROP_BITMASK        (1<<5) /* bitmask of enumerated types */
>>  #define DRM_MODE_PROP_OBJECT (1<<6) /* drm mode object */
>> +/* Properties that are not dynamic cannot safely be changed without a
>> + * atomic-modeset / atomic-pageflip test step.  But if userspace is
>> + * only changing dynamic properties, it is guaranteed that the change
>> + * will not exceed hw limits, so no test step is required.
>> + *
>> + * Note that fb_id properties are a bit ambiguous.. they of course can
>> + * be changed dynamically, assuming the pixel format does not change.
>> + */
>> +#define DRM_MODE_PROP_DYNAMIC        (1<<24)
>
> I'm still not convinced that this is useful. We can't even flag the FB
> ID with this flag since the FB encompasses stride/bpp/etc. parameters
> that for sure can't just be changed w/o first checking the new
> configuration.

well, I know of at least some hw that can change plane src/crtc coords
without requiring a check step.  So I think it is useful as an
optimization.  But it is also something that could be easily/safely
added later so maybe I should drop it from the initial atomic
patchset.

BR,
-R

> --
> Ville Syrjälä
> Intel OTC
diff mbox

Patch

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 35921ba..15db837 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -232,6 +232,15 @@  struct drm_mode_get_connector {
 #define DRM_MODE_PROP_BLOB	(1<<4)
 #define DRM_MODE_PROP_BITMASK	(1<<5) /* bitmask of enumerated types */
 #define DRM_MODE_PROP_OBJECT	(1<<6) /* drm mode object */
+/* Properties that are not dynamic cannot safely be changed without a
+ * atomic-modeset / atomic-pageflip test step.  But if userspace is
+ * only changing dynamic properties, it is guaranteed that the change
+ * will not exceed hw limits, so no test step is required.
+ *
+ * Note that fb_id properties are a bit ambiguous.. they of course can
+ * be changed dynamically, assuming the pixel format does not change.
+ */
+#define DRM_MODE_PROP_DYNAMIC	(1<<24)
 
 struct drm_mode_property_enum {
 	__u64 value;