diff mbox

[v3,1/2] drm/i915/skl+: Check for supported plane configuration in Interlace mode

Message ID 20170630121100.20159-2-mahesh1.kumar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar, Mahesh June 30, 2017, 12:10 p.m. UTC
In Gen9 platform Interlaced fetch mode doesn't support following plane
configuration:
 - Y/Yf tiling
 - 90/270 rotation
 - YUV420 hybrid planar source pixel formats.

This patch adds check to fail the flip if any of the above configuration
is requested.

Changes since V1:
 - handle checks in intel_plane_atomic_check_with_state (ville)
 - takeout plane scaler checks combile with pipe scaler in next patch
Changes since V2:
 - No need to check for NV12 as it need scaling, so it will be rejected
   by scaling check (ville)

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Ville Syrjälä June 30, 2017, 12:26 p.m. UTC | #1
On Fri, Jun 30, 2017 at 05:40:59PM +0530, Mahesh Kumar wrote:
> In Gen9 platform Interlaced fetch mode doesn't support following plane
> configuration:
>  - Y/Yf tiling
>  - 90/270 rotation

The rotation check seems to be missing from the code?

>  - YUV420 hybrid planar source pixel formats.
> 
> This patch adds check to fail the flip if any of the above configuration
> is requested.
> 
> Changes since V1:
>  - handle checks in intel_plane_atomic_check_with_state (ville)
>  - takeout plane scaler checks combile with pipe scaler in next patch
> Changes since V2:
>  - No need to check for NV12 as it need scaling, so it will be rejected
>    by scaling check (ville)
> 
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 4325cb0a04f5..ee76fab7bb6f 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -114,6 +114,8 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
>  	struct drm_i915_private *dev_priv = to_i915(plane->dev);
>  	struct drm_plane_state *state = &intel_state->base;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->base.adjusted_mode;
>  	int ret;
>  
>  	/*
> @@ -173,6 +175,19 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * Y-tiling is not supported in IF-ID Interlace mode in
> +	 * GEN9 and above.
> +	 */
> +	if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable &&
> +	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> +		if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
> +		    state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
> +			DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	/* FIXME pre-g4x don't work like this */
>  	if (intel_state->base.visible)
>  		crtc_state->active_planes |= BIT(intel_plane->id);
> -- 
> 2.13.0
Kumar, Mahesh July 1, 2017, 4:05 a.m. UTC | #2
Hi,


On Friday 30 June 2017 05:56 PM, Ville Syrjälä wrote:
> On Fri, Jun 30, 2017 at 05:40:59PM +0530, Mahesh Kumar wrote:
>> In Gen9 platform Interlaced fetch mode doesn't support following plane
>> configuration:
>>   - Y/Yf tiling
>>   - 90/270 rotation
> The rotation check seems to be missing from the code?
90/270 rotation require Y/Yf tiling, so that will be automagically 
handled as Y-tile case.

-Mahesh
>
>>   - YUV420 hybrid planar source pixel formats.
>>
>> This patch adds check to fail the flip if any of the above configuration
>> is requested.
>>
>> Changes since V1:
>>   - handle checks in intel_plane_atomic_check_with_state (ville)
>>   - takeout plane scaler checks combile with pipe scaler in next patch
>> Changes since V2:
>>   - No need to check for NV12 as it need scaling, so it will be rejected
>>     by scaling check (ville)
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
>> index 4325cb0a04f5..ee76fab7bb6f 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
>> @@ -114,6 +114,8 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
>>   	struct drm_i915_private *dev_priv = to_i915(plane->dev);
>>   	struct drm_plane_state *state = &intel_state->base;
>>   	struct intel_plane *intel_plane = to_intel_plane(plane);
>> +	const struct drm_display_mode *adjusted_mode =
>> +		&crtc_state->base.adjusted_mode;
>>   	int ret;
>>   
>>   	/*
>> @@ -173,6 +175,19 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
>>   	if (ret)
>>   		return ret;
>>   
>> +	/*
>> +	 * Y-tiling is not supported in IF-ID Interlace mode in
>> +	 * GEN9 and above.
>> +	 */
>> +	if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable &&
>> +	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
>> +		if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
>> +		    state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
>> +			DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>>   	/* FIXME pre-g4x don't work like this */
>>   	if (intel_state->base.visible)
>>   		crtc_state->active_planes |= BIT(intel_plane->id);
>> -- 
>> 2.13.0
Ville Syrjälä July 3, 2017, 2:02 p.m. UTC | #3
On Sat, Jul 01, 2017 at 09:35:12AM +0530, Mahesh Kumar wrote:
> Hi,
> 
> 
> On Friday 30 June 2017 05:56 PM, Ville Syrjälä wrote:
> > On Fri, Jun 30, 2017 at 05:40:59PM +0530, Mahesh Kumar wrote:
> >> In Gen9 platform Interlaced fetch mode doesn't support following plane
> >> configuration:
> >>   - Y/Yf tiling
> >>   - 90/270 rotation
> > The rotation check seems to be missing from the code?
> 90/270 rotation require Y/Yf tiling, so that will be automagically 
> handled as Y-tile case.

Right. OK, series lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Was there a bugzilla link we should include in these patches?

> 
> -Mahesh
> >
> >>   - YUV420 hybrid planar source pixel formats.
> >>
> >> This patch adds check to fail the flip if any of the above configuration
> >> is requested.
> >>
> >> Changes since V1:
> >>   - handle checks in intel_plane_atomic_check_with_state (ville)
> >>   - takeout plane scaler checks combile with pipe scaler in next patch
> >> Changes since V2:
> >>   - No need to check for NV12 as it need scaling, so it will be rejected
> >>     by scaling check (ville)
> >>
> >> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++
> >>   1 file changed, 15 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> >> index 4325cb0a04f5..ee76fab7bb6f 100644
> >> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> >> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> >> @@ -114,6 +114,8 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
> >>   	struct drm_i915_private *dev_priv = to_i915(plane->dev);
> >>   	struct drm_plane_state *state = &intel_state->base;
> >>   	struct intel_plane *intel_plane = to_intel_plane(plane);
> >> +	const struct drm_display_mode *adjusted_mode =
> >> +		&crtc_state->base.adjusted_mode;
> >>   	int ret;
> >>   
> >>   	/*
> >> @@ -173,6 +175,19 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
> >>   	if (ret)
> >>   		return ret;
> >>   
> >> +	/*
> >> +	 * Y-tiling is not supported in IF-ID Interlace mode in
> >> +	 * GEN9 and above.
> >> +	 */
> >> +	if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable &&
> >> +	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> >> +		if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
> >> +		    state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
> >> +			DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
> >> +			return -EINVAL;
> >> +		}
> >> +	}
> >> +
> >>   	/* FIXME pre-g4x don't work like this */
> >>   	if (intel_state->base.visible)
> >>   		crtc_state->active_planes |= BIT(intel_plane->id);
> >> -- 
> >> 2.13.0
Kumar, Mahesh July 3, 2017, 3:58 p.m. UTC | #4
Hi,


On Monday 03 July 2017 07:32 PM, Ville Syrjälä wrote:
> On Sat, Jul 01, 2017 at 09:35:12AM +0530, Mahesh Kumar wrote:
>> Hi,
>>
>>
>> On Friday 30 June 2017 05:56 PM, Ville Syrjälä wrote:
>>> On Fri, Jun 30, 2017 at 05:40:59PM +0530, Mahesh Kumar wrote:
>>>> In Gen9 platform Interlaced fetch mode doesn't support following plane
>>>> configuration:
>>>>    - Y/Yf tiling
>>>>    - 90/270 rotation
>>> The rotation check seems to be missing from the code?
>> 90/270 rotation require Y/Yf tiling, so that will be automagically
>> handled as Y-tile case.
> Right. OK, series lgtm
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Was there a bugzilla link we should include in these patches?
following is bugzilla link for the issue.
https://bugs.freedesktop.org/show_bug.cgi?id=90238

-Mahesh
>
>> -Mahesh
>>>>    - YUV420 hybrid planar source pixel formats.
>>>>
>>>> This patch adds check to fail the flip if any of the above configuration
>>>> is requested.
>>>>
>>>> Changes since V1:
>>>>    - handle checks in intel_plane_atomic_check_with_state (ville)
>>>>    - takeout plane scaler checks combile with pipe scaler in next patch
>>>> Changes since V2:
>>>>    - No need to check for NV12 as it need scaling, so it will be rejected
>>>>      by scaling check (ville)
>>>>
>>>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++
>>>>    1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>>> index 4325cb0a04f5..ee76fab7bb6f 100644
>>>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
>>>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>>> @@ -114,6 +114,8 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
>>>>    	struct drm_i915_private *dev_priv = to_i915(plane->dev);
>>>>    	struct drm_plane_state *state = &intel_state->base;
>>>>    	struct intel_plane *intel_plane = to_intel_plane(plane);
>>>> +	const struct drm_display_mode *adjusted_mode =
>>>> +		&crtc_state->base.adjusted_mode;
>>>>    	int ret;
>>>>    
>>>>    	/*
>>>> @@ -173,6 +175,19 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
>>>>    	if (ret)
>>>>    		return ret;
>>>>    
>>>> +	/*
>>>> +	 * Y-tiling is not supported in IF-ID Interlace mode in
>>>> +	 * GEN9 and above.
>>>> +	 */
>>>> +	if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable &&
>>>> +	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
>>>> +		if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
>>>> +		    state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
>>>> +			DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
>>>> +			return -EINVAL;
>>>> +		}
>>>> +	}
>>>> +
>>>>    	/* FIXME pre-g4x don't work like this */
>>>>    	if (intel_state->base.visible)
>>>>    		crtc_state->active_planes |= BIT(intel_plane->id);
>>>> -- 
>>>> 2.13.0
Ville Syrjälä July 4, 2017, 2:41 p.m. UTC | #5
On Mon, Jul 03, 2017 at 09:28:00PM +0530, Mahesh Kumar wrote:
> Hi,
> 
> 
> On Monday 03 July 2017 07:32 PM, Ville Syrjälä wrote:
> > On Sat, Jul 01, 2017 at 09:35:12AM +0530, Mahesh Kumar wrote:
> >> Hi,
> >>
> >>
> >> On Friday 30 June 2017 05:56 PM, Ville Syrjälä wrote:
> >>> On Fri, Jun 30, 2017 at 05:40:59PM +0530, Mahesh Kumar wrote:
> >>>> In Gen9 platform Interlaced fetch mode doesn't support following plane
> >>>> configuration:
> >>>>    - Y/Yf tiling
> >>>>    - 90/270 rotation
> >>> The rotation check seems to be missing from the code?
> >> 90/270 rotation require Y/Yf tiling, so that will be automagically
> >> handled as Y-tile case.
> > Right. OK, series lgtm
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Was there a bugzilla link we should include in these patches?
> following is bugzilla link for the issue.
> https://bugs.freedesktop.org/show_bug.cgi?id=90238

Added that, and pushed the series to dinq. Thanks for the patches.

I had to fix a small checkpatch complaint while applying. Plase always
review the checkpatch warnings when submitting patches.

> 
> -Mahesh
> >
> >> -Mahesh
> >>>>    - YUV420 hybrid planar source pixel formats.
> >>>>
> >>>> This patch adds check to fail the flip if any of the above configuration
> >>>> is requested.
> >>>>
> >>>> Changes since V1:
> >>>>    - handle checks in intel_plane_atomic_check_with_state (ville)
> >>>>    - takeout plane scaler checks combile with pipe scaler in next patch
> >>>> Changes since V2:
> >>>>    - No need to check for NV12 as it need scaling, so it will be rejected
> >>>>      by scaling check (ville)
> >>>>
> >>>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> >>>> ---
> >>>>    drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++
> >>>>    1 file changed, 15 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> >>>> index 4325cb0a04f5..ee76fab7bb6f 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> >>>> @@ -114,6 +114,8 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
> >>>>    	struct drm_i915_private *dev_priv = to_i915(plane->dev);
> >>>>    	struct drm_plane_state *state = &intel_state->base;
> >>>>    	struct intel_plane *intel_plane = to_intel_plane(plane);
> >>>> +	const struct drm_display_mode *adjusted_mode =
> >>>> +		&crtc_state->base.adjusted_mode;
> >>>>    	int ret;
> >>>>    
> >>>>    	/*
> >>>> @@ -173,6 +175,19 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
> >>>>    	if (ret)
> >>>>    		return ret;
> >>>>    
> >>>> +	/*
> >>>> +	 * Y-tiling is not supported in IF-ID Interlace mode in
> >>>> +	 * GEN9 and above.
> >>>> +	 */
> >>>> +	if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable &&
> >>>> +	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> >>>> +		if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
> >>>> +		    state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
> >>>> +			DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
> >>>> +			return -EINVAL;
> >>>> +		}
> >>>> +	}
> >>>> +
> >>>>    	/* FIXME pre-g4x don't work like this */
> >>>>    	if (intel_state->base.visible)
> >>>>    		crtc_state->active_planes |= BIT(intel_plane->id);
> >>>> -- 
> >>>> 2.13.0
Kumar, Mahesh July 5, 2017, 6:39 a.m. UTC | #6
Hi,


On Tuesday 04 July 2017 08:11 PM, Ville Syrjälä wrote:
> On Mon, Jul 03, 2017 at 09:28:00PM +0530, Mahesh Kumar wrote:
>> Hi,
>>
>>
>> On Monday 03 July 2017 07:32 PM, Ville Syrjälä wrote:
>>> On Sat, Jul 01, 2017 at 09:35:12AM +0530, Mahesh Kumar wrote:
>>>> Hi,
>>>>
>>>>
>>>> On Friday 30 June 2017 05:56 PM, Ville Syrjälä wrote:
>>>>> On Fri, Jun 30, 2017 at 05:40:59PM +0530, Mahesh Kumar wrote:
>>>>>> In Gen9 platform Interlaced fetch mode doesn't support following plane
>>>>>> configuration:
>>>>>>     - Y/Yf tiling
>>>>>>     - 90/270 rotation
>>>>> The rotation check seems to be missing from the code?
>>>> 90/270 rotation require Y/Yf tiling, so that will be automagically
>>>> handled as Y-tile case.
>>> Right. OK, series lgtm
>>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Was there a bugzilla link we should include in these patches?
>> following is bugzilla link for the issue.
>> https://bugs.freedesktop.org/show_bug.cgi?id=90238
> Added that, and pushed the series to dinq. Thanks for the patches.
>
> I had to fix a small checkpatch complaint while applying. Plase always
> review the checkpatch warnings when submitting patches.
ok sure, will start reviewing checkpatch warnings.
thanks for review,
Do we need to push these patches to "stable" as well?
for enabling PF-ID Interlace mode what should be the approach?
Should we always enable PF-ID in GEN9+, or should we create property to 
select the fetching mode?
for property based implementation who will be the user-space consumer 
for property?
Please let me know your thoughts on that.
thanks,

-Mahesh
>
>> -Mahesh
>>>> -Mahesh
>>>>>>     - YUV420 hybrid planar source pixel formats.
>>>>>>
>>>>>> This patch adds check to fail the flip if any of the above configuration
>>>>>> is requested.
>>>>>>
>>>>>> Changes since V1:
>>>>>>     - handle checks in intel_plane_atomic_check_with_state (ville)
>>>>>>     - takeout plane scaler checks combile with pipe scaler in next patch
>>>>>> Changes since V2:
>>>>>>     - No need to check for NV12 as it need scaling, so it will be rejected
>>>>>>       by scaling check (ville)
>>>>>>
>>>>>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>>>>>> ---
>>>>>>     drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++
>>>>>>     1 file changed, 15 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>>>>> index 4325cb0a04f5..ee76fab7bb6f 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>>>>> @@ -114,6 +114,8 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
>>>>>>     	struct drm_i915_private *dev_priv = to_i915(plane->dev);
>>>>>>     	struct drm_plane_state *state = &intel_state->base;
>>>>>>     	struct intel_plane *intel_plane = to_intel_plane(plane);
>>>>>> +	const struct drm_display_mode *adjusted_mode =
>>>>>> +		&crtc_state->base.adjusted_mode;
>>>>>>     	int ret;
>>>>>>     
>>>>>>     	/*
>>>>>> @@ -173,6 +175,19 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
>>>>>>     	if (ret)
>>>>>>     		return ret;
>>>>>>     
>>>>>> +	/*
>>>>>> +	 * Y-tiling is not supported in IF-ID Interlace mode in
>>>>>> +	 * GEN9 and above.
>>>>>> +	 */
>>>>>> +	if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable &&
>>>>>> +	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
>>>>>> +		if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
>>>>>> +		    state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
>>>>>> +			DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
>>>>>> +			return -EINVAL;
>>>>>> +		}
>>>>>> +	}
>>>>>> +
>>>>>>     	/* FIXME pre-g4x don't work like this */
>>>>>>     	if (intel_state->base.visible)
>>>>>>     		crtc_state->active_planes |= BIT(intel_plane->id);
>>>>>> -- 
>>>>>> 2.13.0
Ville Syrjälä July 5, 2017, 10:13 a.m. UTC | #7
On Wed, Jul 05, 2017 at 12:09:19PM +0530, Mahesh Kumar wrote:
> Hi,
> 
> 
> On Tuesday 04 July 2017 08:11 PM, Ville Syrjälä wrote:
> > On Mon, Jul 03, 2017 at 09:28:00PM +0530, Mahesh Kumar wrote:
> >> Hi,
> >>
> >>
> >> On Monday 03 July 2017 07:32 PM, Ville Syrjälä wrote:
> >>> On Sat, Jul 01, 2017 at 09:35:12AM +0530, Mahesh Kumar wrote:
> >>>> Hi,
> >>>>
> >>>>
> >>>> On Friday 30 June 2017 05:56 PM, Ville Syrjälä wrote:
> >>>>> On Fri, Jun 30, 2017 at 05:40:59PM +0530, Mahesh Kumar wrote:
> >>>>>> In Gen9 platform Interlaced fetch mode doesn't support following plane
> >>>>>> configuration:
> >>>>>>     - Y/Yf tiling
> >>>>>>     - 90/270 rotation
> >>>>> The rotation check seems to be missing from the code?
> >>>> 90/270 rotation require Y/Yf tiling, so that will be automagically
> >>>> handled as Y-tile case.
> >>> Right. OK, series lgtm
> >>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> Was there a bugzilla link we should include in these patches?
> >> following is bugzilla link for the issue.
> >> https://bugs.freedesktop.org/show_bug.cgi?id=90238
> > Added that, and pushed the series to dinq. Thanks for the patches.
> >
> > I had to fix a small checkpatch complaint while applying. Plase always
> > review the checkpatch warnings when submitting patches.
> ok sure, will start reviewing checkpatch warnings.
> thanks for review,
> Do we need to push these patches to "stable" as well?

I was initially thinking of adding cc:stable, but forgot. In the end
I think this combination might be rare enough that we might not have
to bother unless we get an actual bug report.

> for enabling PF-ID Interlace mode what should be the approach?
> Should we always enable PF-ID in GEN9+, or should we create property to 
> select the fetching mode?

I'm a bit leery on always enabling it becasue that means you can no
longer present interlaced material properly. It sounds a bit unlikely
that anyone would explicitly choose to use an interlaced mode for
progressive material when there's progressive mode available. Not
sure if such crappy TVs exist anymore that can't do eg. 1080p but
can do 1080i. I would hope not.

> for property based implementation who will be the user-space consumer 
> for property?

If it were a connector property then I think X should pick it up
automatically. But would anyone actually use it? I don't know.

> Please let me know your thoughts on that.
> thanks,
> 
> -Mahesh
> >
> >> -Mahesh
> >>>> -Mahesh
> >>>>>>     - YUV420 hybrid planar source pixel formats.
> >>>>>>
> >>>>>> This patch adds check to fail the flip if any of the above configuration
> >>>>>> is requested.
> >>>>>>
> >>>>>> Changes since V1:
> >>>>>>     - handle checks in intel_plane_atomic_check_with_state (ville)
> >>>>>>     - takeout plane scaler checks combile with pipe scaler in next patch
> >>>>>> Changes since V2:
> >>>>>>     - No need to check for NV12 as it need scaling, so it will be rejected
> >>>>>>       by scaling check (ville)
> >>>>>>
> >>>>>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> >>>>>> ---
> >>>>>>     drivers/gpu/drm/i915/intel_atomic_plane.c | 15 +++++++++++++++
> >>>>>>     1 file changed, 15 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> >>>>>> index 4325cb0a04f5..ee76fab7bb6f 100644
> >>>>>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> >>>>>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> >>>>>> @@ -114,6 +114,8 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
> >>>>>>     	struct drm_i915_private *dev_priv = to_i915(plane->dev);
> >>>>>>     	struct drm_plane_state *state = &intel_state->base;
> >>>>>>     	struct intel_plane *intel_plane = to_intel_plane(plane);
> >>>>>> +	const struct drm_display_mode *adjusted_mode =
> >>>>>> +		&crtc_state->base.adjusted_mode;
> >>>>>>     	int ret;
> >>>>>>     
> >>>>>>     	/*
> >>>>>> @@ -173,6 +175,19 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
> >>>>>>     	if (ret)
> >>>>>>     		return ret;
> >>>>>>     
> >>>>>> +	/*
> >>>>>> +	 * Y-tiling is not supported in IF-ID Interlace mode in
> >>>>>> +	 * GEN9 and above.
> >>>>>> +	 */
> >>>>>> +	if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable &&
> >>>>>> +	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> >>>>>> +		if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
> >>>>>> +		    state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
> >>>>>> +			DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
> >>>>>> +			return -EINVAL;
> >>>>>> +		}
> >>>>>> +	}
> >>>>>> +
> >>>>>>     	/* FIXME pre-g4x don't work like this */
> >>>>>>     	if (intel_state->base.visible)
> >>>>>>     		crtc_state->active_planes |= BIT(intel_plane->id);
> >>>>>> -- 
> >>>>>> 2.13.0
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 4325cb0a04f5..ee76fab7bb6f 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -114,6 +114,8 @@  int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv = to_i915(plane->dev);
 	struct drm_plane_state *state = &intel_state->base;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
 	int ret;
 
 	/*
@@ -173,6 +175,19 @@  int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
 	if (ret)
 		return ret;
 
+	/*
+	 * Y-tiling is not supported in IF-ID Interlace mode in
+	 * GEN9 and above.
+	 */
+	if (state->fb && INTEL_GEN(dev_priv) >= 9 && crtc_state->base.enable &&
+	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
+		if (state->fb->modifier == I915_FORMAT_MOD_Y_TILED ||
+		    state->fb->modifier == I915_FORMAT_MOD_Yf_TILED) {
+			DRM_DEBUG_KMS("Y/Yf tiling not supported in IF-ID mode\n");
+			return -EINVAL;
+		}
+	}
+
 	/* FIXME pre-g4x don't work like this */
 	if (intel_state->base.visible)
 		crtc_state->active_planes |= BIT(intel_plane->id);