Re: [RFC PATCH 3/3] drm/i915/display: allow creation of case I915_FORMAT_MOD_4_TILED_XE2_CCS type framebuffer

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

On 7.5.2024 19.06, Ville Syrjälä wrote:
> On Mon, May 06, 2024 at 09:52:38PM +0300, Juha-Pekka Heikkila wrote:
>> Add I915_FORMAT_MOD_4_TILED_XE2_CCS to possible created tiling for new framebuffer
>> on Xe driver.
>>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c       |  1 +
>>   drivers/gpu/drm/i915/display/intel_fb.c            | 10 ++++++++++
>>   drivers/gpu/drm/i915/display/skl_universal_plane.c |  4 +++-
>>   3 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index ef986b508431..083147a21edb 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -6150,6 +6150,7 @@ static int intel_async_flip_check_hw(struct intel_atomic_state *state, struct in
>>   		case I915_FORMAT_MOD_Y_TILED:
>>   		case I915_FORMAT_MOD_Yf_TILED:
>>   		case I915_FORMAT_MOD_4_TILED:
>> +		case I915_FORMAT_MOD_4_TILED_XE2_CCS:
>>   			break;
>>   		default:
>>   			drm_dbg_kms(&i915->drm,
>> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
>> index bf24f48a1e76..6a44746b8381 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
>> @@ -161,6 +161,10 @@ struct intel_modifier_desc {
>>   
>>   static const struct intel_modifier_desc intel_modifiers[] = {
>>   	{
>> +		.modifier = I915_FORMAT_MOD_4_TILED_XE2_CCS,
>> +		.display_ver = { 14, -1 },
> 
> Should that say 20?

No, 14 is smallest display version where this can be used. (and graphics 
version 20)

> 
>> +		.plane_caps = INTEL_PLANE_CAP_TILING_4,
>> +	}, {
>>   		.modifier = I915_FORMAT_MOD_4_TILED_MTL_MC_CCS,
>>   		.display_ver = { 14, 14 },
>>   		.plane_caps = INTEL_PLANE_CAP_TILING_4 | INTEL_PLANE_CAP_CCS_MC,
>> @@ -435,6 +439,10 @@ static bool plane_has_modifier(struct drm_i915_private *i915,
>>   	    HAS_FLAT_CCS(i915) != !md->ccs.packed_aux_planes)
>>   		return false;
>>   
>> +	if (md->modifier == I915_FORMAT_MOD_4_TILED_XE2_CCS &&
>> +	    GRAPHICS_VER(i915) < 20)
>> +		return false;
>> +
>>   	return true;
>>   }
>>   
>> @@ -657,6 +665,7 @@ intel_tile_width_bytes(const struct drm_framebuffer *fb, int color_plane)
>>   			return 128;
>>   		else
>>   			return 512;
>> +	case I915_FORMAT_MOD_4_TILED_XE2_CCS:
>>   	case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
>>   	case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS_CC:
>>   	case I915_FORMAT_MOD_4_TILED_DG2_MC_CCS:
>> @@ -858,6 +867,7 @@ unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
>>   	case I915_FORMAT_MOD_4_TILED_MTL_RC_CCS:
>>   	case I915_FORMAT_MOD_4_TILED_MTL_RC_CCS_CC:
>>   		return 16 * 1024;
>> +	case I915_FORMAT_MOD_4_TILED_XE2_CCS:
>>   	case I915_FORMAT_MOD_Y_TILED_CCS:
>>   	case I915_FORMAT_MOD_Yf_TILED_CCS:
>>   	case I915_FORMAT_MOD_Y_TILED:
>> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> index 0a8e781a3648..e590fea1180a 100644
>> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
>> @@ -792,6 +792,7 @@ static u32 skl_plane_ctl_tiling(u64 fb_modifier)
>>   	case I915_FORMAT_MOD_Y_TILED:
>>   		return PLANE_CTL_TILED_Y;
>>   	case I915_FORMAT_MOD_4_TILED:
>> +	case I915_FORMAT_MOD_4_TILED_XE2_CCS:
>>   		return PLANE_CTL_TILED_4;
>>   	case I915_FORMAT_MOD_4_TILED_DG2_RC_CCS:
>>   		return PLANE_CTL_TILED_4 |
>> @@ -949,7 +950,8 @@ static u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
>>   		plane_ctl |= adlp_plane_ctl_arb_slots(plane_state);
>>   
>>   	if (GRAPHICS_VER(dev_priv) >= 20 &&
>> -	    fb->modifier == I915_FORMAT_MOD_4_TILED) {
>> +	    (fb->modifier == I915_FORMAT_MOD_4_TILED ||
>> +	     fb->modifier == I915_FORMAT_MOD_4_TILED_XE2_CCS)) {
>>   		plane_ctl |= PLANE_CTL_RENDER_DECOMPRESSION_ENABLE;
> 
> If we got to the trouble of adding a new modifier then we should
> just let skl_plane_ctl_tiling() handle this, like it does for
> every other platform.

yea, this will need to be moved. Otherwise here start to gather clutter.

> 
>>   	}
>>   
>> -- 
>> 2.43.2
>