[Intel-gfx] [PATCH 1/2] drm/i915: Relocate SKL+ NV12 src width w/a

[Maarten Lankhorst]

Op 23-10-18 om 15:40 schreef Ville Syrjälä:
> On Tue, Oct 23, 2018 at 03:25:03PM +0200, Maarten Lankhorst wrote:
>> Op 18-10-18 om 21:59 schreef Ville Syrjala:
>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>
>>> The SKL+ NV12 src width alignment w/a is still living in an odd place.
>>> Everything else was already relocated closer to the main plane check
>>> function. Move this workaround as well.
>>>
>>> As a bonus we avoid the funky rotated vs. not mess with the src
>>> coordinates as this now gets checked before we rotate the coordinates.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_display.c | 25 -------------------------
>>>  drivers/gpu/drm/i915/intel_sprite.c  | 21 +++++++++++++++++++++
>>>  2 files changed, 21 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index fc7e3b0bd95c..940d514cf9d7 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -3049,28 +3049,6 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>>>  	return 0;
>>>  }
>>>  
>>> -static int
>>> -skl_check_nv12_surface(struct intel_plane_state *plane_state)
>>> -{
>>> -	/* Display WA #1106 */
>>> -	if (plane_state->base.rotation !=
>>> -	    (DRM_MODE_REFLECT_X | DRM_MODE_ROTATE_90) &&
>>> -	    plane_state->base.rotation != DRM_MODE_ROTATE_270)
>>> -		return 0;
>>> -
>>> -	/*
>>> -	 * src coordinates are rotated here.
>>> -	 * We check height but report it as width
>>> -	 */
>>> -	if (((drm_rect_height(&plane_state->base.src) >> 16) % 4) != 0) {
>>> -		DRM_DEBUG_KMS("src width must be multiple "
>>> -			      "of 4 for rotated NV12\n");
>>> -		return -EINVAL;
>>> -	}
>>> -
>>> -	return 0;
>>> -}
>>> -
>>>  static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
>>>  {
>>>  	const struct drm_framebuffer *fb = plane_state->base.fb;
>>> @@ -3153,9 +3131,6 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
>>>  	 * the main surface setup depends on it.
>>>  	 */
>>>  	if (fb->format->format == DRM_FORMAT_NV12) {
>>> -		ret = skl_check_nv12_surface(plane_state);
>>> -		if (ret)
>>> -			return ret;
>>>  		ret = skl_check_nv12_aux_surface(plane_state);
>>>  		if (ret)
>>>  			return ret;
>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>>> index 7cd59eee5cad..0fe6c664e83c 100644
>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>> @@ -1342,6 +1342,23 @@ static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_s
>>>  	return 0;
>>>  }
>>>  
>>> +static int skl_plane_check_nv12_rotation(const struct intel_plane_state *plane_state)
>>> +{
>>> +	const struct drm_framebuffer *fb = plane_state->base.fb;
>>> +	unsigned int rotation = plane_state->base.rotation;
>>> +	int src_w = drm_rect_width(&plane_state->base.src) >> 16;
>>> +
>>> +	/* Display WA #1106 */
>>> +	if (fb->format->format == DRM_FORMAT_NV12 && src_w & 3 &&
>> Could we put more nv12 checks here? I want to pull the scaler SRC W/H checks in as well..
> Hmm. The way those are done atm is a bit funky, but there isa the 
> nv12 vs. not-nv12 limits to consider there as well. Not really sure
> what the best approach is.
>
>> Probably stylize it a bit with an early return for !NV12, so all checks come after that. :)
>>
>> ~Maarten
>>
>> Other than that patch 1-2 look good, so have a r-b with or without those suggested changes.
> Thanks. I think I'll smash these in as is for now and think a bit more
> about the scaler stuff.
Was thinking of keeping the generic scaler limits in there, and make them more strict in the above check..

That way we can run the checks for ICL nv12 as well..