[Intel-gfx] [PATCH 1/2] drm/i915/skl+: Check for supported plane configuration in Interlace mode

[Mahesh Kumar]

Hi,


On Thursday 29 June 2017 07:36 PM, Ville Syrjälä wrote:
> On Thu, Jun 29, 2017 at 02:27:39PM +0530, Mahesh Kumar wrote:
>> In Gen9 platform Interlaced fetch mode doesn't support following plane
>> configuration:
>>   - Y/Yf tiling
>>   - 90/270 rotation
>>   - Scaling
>>   - YUV420 hybrid planar source pixel formats.
>>
>> This patch adds check to fail the flip if any of the above configuration
>> is requested.
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 4e03ca6c946f..1f2394a0c07d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11022,6 +11022,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> calc_changes seems like the wrong place for this.
> intel_plane_atomic_check_with_state() seems better.
ok, will move it to intel_plane_atomic_check_with_state()
>
>>   	bool is_crtc_enabled = crtc_state->active;
>>   	bool turn_off, turn_on, visible, was_visible;
>>   	struct drm_framebuffer *fb = plane_state->fb;
>> +	const struct drm_display_mode *mode = &crtc_state->adjusted_mode;
> Please call it "adjusted_mode".
ok :)
>
>>   	int ret;
>>   
>>   	if (INTEL_GEN(dev_priv) >= 9 && plane->id != PLANE_CURSOR) {
>> @@ -11108,6 +11109,41 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>   	    !needs_scaling(old_plane_state))
>>   		pipe_config->disable_lp_wm = true;
>>   
>> +	/*
>> +	 * Y-tiling is not supported in IF-ID Interlace mode in
>> +	 * GEN9 and above.
>> +	 * Scaling is not supported with Interlaced fetch mode.
>> +	 * YUV420 hybrid planar source pixel formats are not supported with
>> +	 * Interlaced fetch mode.
>> +	 */
>> +	if (visible && INTEL_GEN(dev_priv) >= 9 &&
> I think here we probably want state->fb instead of visible.
> Hmm. I guess it'll have to check for crtc_state->enable as well
> to make sure the adjusted_mode is valid.
>
> In fact it looks to me like we could perhaps combine these with
> the already existing rotation vs. format/modifier checks in
> intel_plane_atomic_check_with_state()
ok
thanks for review

-Mahesh
>
>> +				mode->flags & DRM_MODE_FLAG_INTERLACE) {
>> +		struct drm_format_name_buf format_name;
>> +
>> +		if (fb->modifier == I915_FORMAT_MOD_Y_TILED ||
>> +		    fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
>> +			DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		if (needs_scaling(to_intel_plane_state(plane_state))) {
>> +			DRM_DEBUG_KMS("Scaling not supported in IF-ID mode\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		switch (fb->format->format) {
>> +		case DRM_FORMAT_NV12:
>> +		case DRM_FORMAT_YUV420:
>> +		case DRM_FORMAT_YVU420:
> We don't support those formats. NV12 we'll hopefully get some
> time soon, so we could keep that.
>
>> +			DRM_DEBUG_KMS("Unsupported pixel format %s for IF-ID\n",
>> +				      drm_get_format_name(fb->format->format,
>> +				      &format_name));
>> +			return -EINVAL;
>> +		default:
>> +			break;
>> +		}
>> +	}
>> +
>>   	return 0;
>>   }
>>   
>> -- 
>> 2.13.0