Re: [PATCH i-g-t v5] lib/igt_kms: Add COMIT_ATOMIC to igt_display_commit2()

[Pratik Vishwakarma <pratik.vishwakarma@intel.com>]

Hi Maarten,

There will be no difference in execution based on ATOMIC client cap.
If it fails, the number of properties we receive is 2 (type and 
rotation) whereas if ATOMIC client cap is set, we actually receive all 
the supported atomic properties i.e. src_w,src_h etc.

We have added the return check in igt_atomic_commit.

We tested this patch by modifying the kms_rotation_crc test and found 
that this is required.
I will be submitting the kms_rotation_crc patch separately.

Let me know your thoughts on this.

P.S. I will submit a new patch with Jani's comments fix.

On Monday 15 February 2016 06:12 PM, Maarten Lankhorst wrote:
> Op 12-02-16 om 11:34 schreef Pratik Vishwakarma:
>> From: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
>>
>> Co-Author : Marius Vlad <marius.c.vlad@intel.com>
>> Co-Author : Pratik Vishwakarma <pratik.vishwakarma@intel.com>
>>
>> So far we have had only two commit styles, COMMIT_LEGACY
>> and COMMIT_UNIVERSAL. This patch adds another commit style
>> COMMIT_ATOMIC which makes use of drmModeAtomicCommit()
>>
>> v2: (Marius)
>> 	i)Set CRTC_ID to zero while disabling plane
>> 	ii)Modified the log message in igt_atomic_prepare_plane_commit
>> 	https://patchwork.freedesktop.org/patch/71945/
>>
>> v3: (Marius)Set FB_ID to zero while disabling plane
>> 	https://patchwork.freedesktop.org/patch/72179/
>>
>> v4: (Maarten) Corrected the typo in commit message
>> 	https://patchwork.freedesktop.org/patch/72598/
>>
>> v5: Added check for DRM_CLIENT_CAP_ATOMIC in igt_display_init
>>      (Marius)
>> 	i)Removed unused props from igt_display_init
>> 	ii)Removed unused ret. Changed function to void
>> 	iii)Declare the variable before checking if we have
>> 	DRM_CLIENT_CAP_ATOMIC.
>>
>> Signed-off-by: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
>> Signed-off-by: Pratik Vishwakarma <pratik.vishwakarma@intel.com>
>> ---
>>   lib/igt_kms.c | 317 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   lib/igt_kms.h |  71 ++++++++++++-
>>   2 files changed, 386 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 90c8da7..8e201e8 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -145,6 +145,120 @@ const unsigned char* igt_kms_get_base_edid(void)
>>    *
>>    * Returns: an alternate edid block
>>    */
>> +static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
>> +	"SRC_X",
>> +	"SRC_Y",
>> +	"SRC_W",
>> +	"SRC_H",
>> +	"CRTC_X",
>> +	"CRTC_Y",
>> +	"CRTC_W",
>> +	"CRTC_H",
>> +	"FB_ID",
>> +	"CRTC_ID",
>> +	"type",
>> +	"rotation"
>> +};
>> +
>> +static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
>> +	"background_color"
>> +};
>> +
>> +static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
>> +	"scaling mode",
>> +	"DPMS"
>> +};
>> +
>> +/*
>> + * Retrieve all the properies specified in props_name and store them into
>> + * plane->atomic_props_plane.
>> + */
>> +static void
>> +igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
>> +			int num_props, const char **prop_names)
>> +{
>> +	drmModeObjectPropertiesPtr props;
>> +	int i, j, fd;
>> +
>> +	fd = display->drm_fd;
>> +
>> +	props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, DRM_MODE_OBJECT_PLANE);
>> +	igt_assert(props);
>> +
>> +	for (i = 0; i < props->count_props; i++) {
>> +		drmModePropertyPtr prop =
>> +			drmModeGetProperty(fd, props->props[i]);
>> +
>> +		for (j = 0; j < num_props; j++) {
>> +			if (strcmp(prop->name, prop_names[j]) != 0)
>> +				continue;
>> +
>> +			plane->atomic_props_plane[j] = props->props[i];
>> +			break;
>> +		}
>> +
>> +		drmModeFreeProperty(prop);
>> +	}
>> +
>> +	drmModeFreeObjectProperties(props);
>> +}
>> +
>> +/*
>> + * Retrieve all the properies specified in props_name and store them into
>> + * config->atomic_props_crtc and config->atomic_props_connector.
>> + */
>> +static void
>> +igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
>> +			int num_crtc_props, const char **crtc_prop_names,
>> +			int num_connector_props, const char **conn_prop_names)
>> +{
>> +	drmModeObjectPropertiesPtr props;
>> +	int i, j, fd;
>> +
>> +	fd = display->drm_fd;
>> +
>> +	props = drmModeObjectGetProperties(fd, output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC);
>> +	igt_assert(props);
>> +
>> +	for (i = 0; i < props->count_props; i++) {
>> +		drmModePropertyPtr prop =
>> +			drmModeGetProperty(fd, props->props[i]);
>> +
>> +		for (j = 0; j < num_crtc_props; j++) {
>> +			if (strcmp(prop->name, crtc_prop_names[j]) != 0)
>> +				continue;
>> +
>> +			output->config.atomic_props_crtc[j] = props->props[i];
>> +			break;
>> +		}
>> +
>> +		drmModeFreeProperty(prop);
>> +	}
>> +
>> +	drmModeFreeObjectProperties(props);
>> +	props = NULL;
>> +	props = drmModeObjectGetProperties(fd, output->config.connector->connector_id, DRM_MODE_OBJECT_CONNECTOR);
>> +	igt_assert(props);
>> +
>> +	for (i = 0; i < props->count_props; i++) {
>> +		drmModePropertyPtr prop =
>> +			drmModeGetProperty(fd, props->props[i]);
>> +
>> +		for (j = 0; j < num_connector_props; j++) {
>> +			if (strcmp(prop->name, conn_prop_names[j]) != 0)
>> +				continue;
>> +
>> +			output->config.atomic_props_connector[j] = props->props[i];
>> +			break;
>> +		}
>> +
>> +		drmModeFreeProperty(prop);
>> +	}
>> +
>> +	drmModeFreeObjectProperties(props);
>> +
>> +}
>> +
>>   const unsigned char* igt_kms_get_alt_edid(void)
>>   {
>>   	update_edid_csum(alt_edid);
>> @@ -952,6 +1066,8 @@ static void igt_output_refresh(igt_output_t *output)
>>   	    kmstest_pipe_name(output->config.pipe));
>>   
>>   	display->pipes_in_use |= 1 << output->config.pipe;
>> +	igt_atomic_fill_props(display, output, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names,
>> +		IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
>>   }
>>   
>>   static bool
>> @@ -1038,6 +1154,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>>   	display->n_pipes = resources->count_crtcs;
>>   
>>   	drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
>> +	drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
>>   	plane_resources = drmModeGetPlaneResources(display->drm_fd);
>>   	igt_assert(plane_resources);
> Since atomic isn't enabled yet by default in i915, would it make sense if the return value for this cap was checked to see if atomic is available?
> And what Jani says, typo still isn't fixed. ;-)
>
> ~Maarten
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx