Re: [igt-dev] [PATCH i-g-t 17/25] lib/igt_fb: Remove the hand rolled addfb2

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Qui, 2018-07-19 às 18:04 +0300, Ville Syrjala escreveu:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Just use __kms_addfb() always instead of hand rolling another
> drmModeAddFB2() usage. __kms_addfb() demands support for modifiers
> but we've had that for a long time now so it's not a problem.

Dear IGT mainainers, do we have any specific rules/requirements about
running new IGT in older Kernels? A minimum/maximum age?

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_fb.c | 35 +++++++----------------------------
>  1 file changed, 7 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 77611b593a7e..648f0a318c91 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -856,6 +856,7 @@ igt_create_fb_with_bo_size(int fd, int width, int
> height,
>  	enum igt_color_encoding color_encoding =
> IGT_COLOR_YCBCR_BT709;
>  	enum igt_color_range color_range =
> IGT_COLOR_YCBCR_LIMITED_RANGE;
>  	struct format_desc_struct *f = lookup_drm_format(format);
> +	uint32_t pitches[4];
>  	uint32_t fb_id;
>  	int i;
>  
> @@ -875,35 +876,13 @@ igt_create_fb_with_bo_size(int fd, int width,
> int height,
>  	igt_debug("%s(handle=%d, pitch=%d)\n",
>  		  __func__, fb->gem_handle, fb->stride);
>  
> -	if (tiling != LOCAL_DRM_FORMAT_MOD_NONE &&
> -	    tiling != LOCAL_I915_FORMAT_MOD_X_TILED) {
> -		uint32_t pitches[4];
> -
> -		for (i = 0; i < fb_num_planes(f); i++)
> -			pitches[i] = fb->stride;
> -
> -		do_or_die(__kms_addfb(fd, fb->gem_handle, width,
> height,
> -				      format, tiling, pitches, fb-
> >offsets,
> -				      fb_num_planes(f),
> -				      LOCAL_DRM_MODE_FB_MODIFIERS,
> &fb_id));
> -	} else {
> -		uint32_t handles[4];
> -		uint32_t pitches[4];
> +	for (i = 0; i < fb_num_planes(f); i++)
> +		pitches[i] = fb->stride;
>  
> -		memset(handles, 0, sizeof(handles));
> -		memset(pitches, 0, sizeof(pitches));
> -
> -		handles[0] = fb->gem_handle;
> -		pitches[0] = fb->stride;
> -		for (i = 0; i < fb_num_planes(f); i++) {
> -			handles[i] = fb->gem_handle;
> -			pitches[i] = fb->stride;
> -		}
> -
> -		do_or_die(drmModeAddFB2(fd, width, height, format,
> -					handles, pitches, fb-
> >offsets,
> -					&fb_id, 0));
> -	}
> +	do_or_die(__kms_addfb(fd, fb->gem_handle, width, height,
> +			      format, tiling, pitches, fb->offsets,

So the real difference is that we now pass "tiling" as a modifier as
opposed to nothing, for the X and linear formats. A quick look at
intel_framebuffer_init() suggests that this is fine (and required).

As long as maintainers don't block it:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

But now __kms_addfb() is almost a reimplementation of of
drmModeAddFB2WithModifiers(), maybe we could use it instead.

> +			      fb_num_planes(f),
> +			      LOCAL_DRM_MODE_FB_MODIFIERS, &fb_id));
>  
>  	fb->width = width;
>  	fb->height = height;
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev