Re: [PATCH 3/4] drm/i915: Use frame buffer modifiers for tiled display

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 02/03/2015 07:47 PM, Daniel Vetter wrote:
> On Tue, Feb 03, 2015 at 05:22:31PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Start using frame buffer modifiers instead of object tiling mode
>> for display purposes.
>>
>> To ensure compatibility with old userspace which is using set_tiling
>> and does not know about frame buffer modifiers, the latter are faked
>> internally when tile object is set for display. This way all interested
>> call sites can use fb modifiers exclusively.
>>
>> Also ensure tiling specified via fb modifiers must match object tiling
>> used for fencing if both are specified.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 95 +++++++++++++++++++++++++-----------
>>   drivers/gpu/drm/i915/intel_drv.h     |  2 +
>>   drivers/gpu/drm/i915/intel_pm.c      |  7 +--
>>   drivers/gpu/drm/i915/intel_sprite.c  | 26 +++++-----
>>   4 files changed, 85 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 7a3ed61..6825016 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2198,6 +2198,19 @@ intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling)
>>   	return ALIGN(height, tile_height);
>>   }
>>
>> +static unsigned int intel_fb_modifier_to_tiling(u64 mod)
>> +{
>> +	BUILD_BUG_ON((I915_FORMAT_MOD_X_TILED & 0x00ffffffffffffffL) !=
>> +		     I915_TILING_X);
>> +
>> +	return mod & 1;
>> +}
>> +
>> +unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb)
>> +{
>> +	return intel_fb_modifier_to_tiling(fb->modifier[0]);
>> +}
>
> I expect that these here will create a bit of churn with the skl patches
> you have based, since I really don't want a new I915_TILING_FANCY define
> in the enum space used by obj->tiling mode. But makes sense for backwards
> compat with older platforms and less churn in code.

I thought we talked about effectively creating a new enum space for fb 
tiling? By masking out bits from the fb modifier, no? Only thing for 
backward compatibility is that object X tiling and fb X tiling == 1.

> With igt for the new cases in addfb and review this is imo good to get in.

I can do the IGT, but who is doing the libdrm part? :)

Regards,

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