diff mbox series

[1/5] drm/i915/display: Fix the plane max height and width limits

Message ID 20241025060136.9884-2-suraj.kandpal@intel.com (mailing list archive)
State New
Headers show
Series Add 6k resolution support for a single CRTC | expand

Commit Message

Kandpal, Suraj Oct. 25, 2024, 6:01 a.m. UTC
Fix the plane max height and width limits taking into account the
joined pipe limits too.

Bspec: 28692, 49199, 68858
Fixes: 63dc014e37b9 ("drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3.")
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

Comments

Ville Syrjälä Oct. 25, 2024, 6:21 a.m. UTC | #1
On Fri, Oct 25, 2024 at 11:31:32AM +0530, Suraj Kandpal wrote:
> Fix the plane max height and width limits taking into account the
> joined pipe limits too.
> 
> Bspec: 28692, 49199, 68858
> Fixes: 63dc014e37b9 ("drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3.")
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index ef1436146325..fc578af4f394 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8450,9 +8450,22 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
>  	 * plane so let's not advertize modes that are
>  	 * too big for that.
>  	 */
> -	if (DISPLAY_VER(dev_priv) >= 11) {
> -		plane_width_max = 5120 * num_joined_pipes;
> -		plane_height_max = 4320;
> +	if (DISPLAY_VER(dev_priv) >= 20) {
> +		if (num_joined_pipes > 1) {
> +			plane_width_max = 8192;
> +			plane_height_max = 4800;
> +		} else {
> +			plane_width_max = 5120;
> +			plane_height_max = 4096;

The joiner operates on horizontal lines. Why would the
vertical resolution change here?

And this is breaking ultrajoiner now.

Frankly I have a hard time believing that there are any extra
limits on plane size imposed by the hardware for joined pipes.
If there is some kind of maximum width limit then it must be
coming from the joiner itself (eg. max line buffer width) and
not from the planes. So I think this is the wrong place to
handle that.

> +		}
> +	} else if (DISPLAY_VER(dev_priv) >= 11) {
> +		if (num_joined_pipes > 1) {
> +			plane_width_max = 7680;
> +			plane_height_max = 4320;
> +		} else {
> +			plane_width_max = 5120;
> +			plane_height_max = 4096;
> +		}
>  	} else {
>  		plane_width_max = 5120;
>  		plane_height_max = 4096;
> -- 
> 2.34.1
Kandpal, Suraj Oct. 25, 2024, 6:26 a.m. UTC | #2
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Friday, October 25, 2024 11:51 AM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Nautiyal,
> Ankit K <ankit.k.nautiyal@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: Re: [PATCH 1/5] drm/i915/display: Fix the plane max height and width
> limits
> 
> On Fri, Oct 25, 2024 at 11:31:32AM +0530, Suraj Kandpal wrote:
> > Fix the plane max height and width limits taking into account the
> > joined pipe limits too.
> >
> > Bspec: 28692, 49199, 68858
> > Fixes: 63dc014e37b9 ("drm/i915/dp: Allow big joiner modes in
> > intel_dp_mode_valid(), v3.")
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index ef1436146325..fc578af4f394 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -8450,9 +8450,22 @@ intel_mode_valid_max_plane_size(struct
> drm_i915_private *dev_priv,
> >  	 * plane so let's not advertize modes that are
> >  	 * too big for that.
> >  	 */
> > -	if (DISPLAY_VER(dev_priv) >= 11) {
> > -		plane_width_max = 5120 * num_joined_pipes;
> > -		plane_height_max = 4320;
> > +	if (DISPLAY_VER(dev_priv) >= 20) {
> > +		if (num_joined_pipes > 1) {
> > +			plane_width_max = 8192;
> > +			plane_height_max = 4800;
> > +		} else {
> > +			plane_width_max = 5120;
> > +			plane_height_max = 4096;
> 
> The joiner operates on horizontal lines. Why would the vertical resolution
> change here?
> 

This is what I got from the bspec when pipe are joined the vertical resolution ends up
Changing 

Regards,
Suraj Kandpal

> And this is breaking ultrajoiner now.
> 
> Frankly I have a hard time believing that there are any extra limits on plane
> size imposed by the hardware for joined pipes.
> If there is some kind of maximum width limit then it must be coming from the
> joiner itself (eg. max line buffer width) and not from the planes. So I think this
> is the wrong place to handle that.
> 
> > +		}
> > +	} else if (DISPLAY_VER(dev_priv) >= 11) {
> > +		if (num_joined_pipes > 1) {
> > +			plane_width_max = 7680;
> > +			plane_height_max = 4320;
> > +		} else {
> > +			plane_width_max = 5120;
> > +			plane_height_max = 4096;
> > +		}
> >  	} else {
> >  		plane_width_max = 5120;
> >  		plane_height_max = 4096;
> > --
> > 2.34.1
> 
> --
> Ville Syrjälä
> Intel
Ville Syrjälä Oct. 25, 2024, 7:15 a.m. UTC | #3
On Fri, Oct 25, 2024 at 06:26:10AM +0000, Kandpal, Suraj wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Friday, October 25, 2024 11:51 AM
> > To: Kandpal, Suraj <suraj.kandpal@intel.com>
> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Nautiyal,
> > Ankit K <ankit.k.nautiyal@intel.com>; Roper, Matthew D
> > <matthew.d.roper@intel.com>
> > Subject: Re: [PATCH 1/5] drm/i915/display: Fix the plane max height and width
> > limits
> > 
> > On Fri, Oct 25, 2024 at 11:31:32AM +0530, Suraj Kandpal wrote:
> > > Fix the plane max height and width limits taking into account the
> > > joined pipe limits too.
> > >
> > > Bspec: 28692, 49199, 68858
> > > Fixes: 63dc014e37b9 ("drm/i915/dp: Allow big joiner modes in
> > > intel_dp_mode_valid(), v3.")
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 19 ++++++++++++++++---
> > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index ef1436146325..fc578af4f394 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -8450,9 +8450,22 @@ intel_mode_valid_max_plane_size(struct
> > drm_i915_private *dev_priv,
> > >  	 * plane so let's not advertize modes that are
> > >  	 * too big for that.
> > >  	 */
> > > -	if (DISPLAY_VER(dev_priv) >= 11) {
> > > -		plane_width_max = 5120 * num_joined_pipes;
> > > -		plane_height_max = 4320;
> > > +	if (DISPLAY_VER(dev_priv) >= 20) {
> > > +		if (num_joined_pipes > 1) {
> > > +			plane_width_max = 8192;
> > > +			plane_height_max = 4800;
> > > +		} else {
> > > +			plane_width_max = 5120;
> > > +			plane_height_max = 4096;
> > 
> > The joiner operates on horizontal lines. Why would the vertical resolution
> > change here?
> > 
> 
> This is what I got from the bspec when pipe are joined the vertical resolution ends up
> Changing 

Those bspec pages look very confused.
Nautiyal, Ankit K Oct. 25, 2024, 7:24 a.m. UTC | #4
On 10/25/2024 11:51 AM, Ville Syrjälä wrote:
> On Fri, Oct 25, 2024 at 11:31:32AM +0530, Suraj Kandpal wrote:
>> Fix the plane max height and width limits taking into account the
>> joined pipe limits too.
>>
>> Bspec: 28692, 49199, 68858
>> Fixes: 63dc014e37b9 ("drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3.")
>> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c | 19 ++++++++++++++++---
>>   1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index ef1436146325..fc578af4f394 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -8450,9 +8450,22 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
>>   	 * plane so let's not advertize modes that are
>>   	 * too big for that.
>>   	 */
>> -	if (DISPLAY_VER(dev_priv) >= 11) {
>> -		plane_width_max = 5120 * num_joined_pipes;
>> -		plane_height_max = 4320;
>> +	if (DISPLAY_VER(dev_priv) >= 20) {
>> +		if (num_joined_pipes > 1) {
>> +			plane_width_max = 8192;
>> +			plane_height_max = 4800;
>> +		} else {
>> +			plane_width_max = 5120;
>> +			plane_height_max = 4096;
> The joiner operates on horizontal lines. Why would the
> vertical resolution change here?
>
> And this is breaking ultrajoiner now.

Yeah this will complicate function to check whether go to ultrajoiner or 
bigjoiner.

Perhaps need to have separate function for max_joined_plane_width() and 
max_unjoined_plane_width();

And the func intel_dp_needs_joiner() will change to something like:

bool intel_dp_needs_joiner(struct intel_dp *intel_dp,
                            struct intel_connector *connector,
                            int hdisplay, int clock,
                            int num_joined_pipes)
{
         struct drm_i915_private *i915 = dp_to_i915(intel_dp);

         if (!intel_dp_has_joiner(intel_dp))
                 return false;

         if (hdisplay > max_joined_plane_width())
                 return false;

         if (num_joined_pipes == 2)
                 return clock > i915->display.cdclk.max_dotclk_freq ||
                         hdisplay > max_unjoined_plane_width();
         if (num_joined_pipes == 4)
                 return clock > 2 * i915->display.cdclk.max_dotclk_freq;

         return false;
}

Regards,

Ankit


>
> Frankly I have a hard time believing that there are any extra
> limits on plane size imposed by the hardware for joined pipes.
> If there is some kind of maximum width limit then it must be
> coming from the joiner itself (eg. max line buffer width) and
> not from the planes. So I think this is the wrong place to
> handle that.
>
>> +		}
>> +	} else if (DISPLAY_VER(dev_priv) >= 11) {
>> +		if (num_joined_pipes > 1) {
>> +			plane_width_max = 7680;
>> +			plane_height_max = 4320;
>> +		} else {
>> +			plane_width_max = 5120;
>> +			plane_height_max = 4096;
>> +		}
>>   	} else {
>>   		plane_width_max = 5120;
>>   		plane_height_max = 4096;
>> -- 
>> 2.34.1
Ville Syrjälä Oct. 25, 2024, 8:04 a.m. UTC | #5
On Fri, Oct 25, 2024 at 12:54:45PM +0530, Nautiyal, Ankit K wrote:
> 
> On 10/25/2024 11:51 AM, Ville Syrjälä wrote:
> > On Fri, Oct 25, 2024 at 11:31:32AM +0530, Suraj Kandpal wrote:
> >> Fix the plane max height and width limits taking into account the
> >> joined pipe limits too.
> >>
> >> Bspec: 28692, 49199, 68858
> >> Fixes: 63dc014e37b9 ("drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3.")
> >> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_display.c | 19 ++++++++++++++++---
> >>   1 file changed, 16 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index ef1436146325..fc578af4f394 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -8450,9 +8450,22 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> >>   	 * plane so let's not advertize modes that are
> >>   	 * too big for that.
> >>   	 */
> >> -	if (DISPLAY_VER(dev_priv) >= 11) {
> >> -		plane_width_max = 5120 * num_joined_pipes;
> >> -		plane_height_max = 4320;
> >> +	if (DISPLAY_VER(dev_priv) >= 20) {
> >> +		if (num_joined_pipes > 1) {
> >> +			plane_width_max = 8192;
> >> +			plane_height_max = 4800;
> >> +		} else {
> >> +			plane_width_max = 5120;
> >> +			plane_height_max = 4096;
> > The joiner operates on horizontal lines. Why would the
> > vertical resolution change here?
> >
> > And this is breaking ultrajoiner now.
> 
> Yeah this will complicate function to check whether go to ultrajoiner or 
> bigjoiner.
> 
> Perhaps need to have separate function for max_joined_plane_width() and 
> max_unjoined_plane_width();

I don't think max plane sizes have anything to with joiner. In fact
we should probably make this code just ask the "primary" plane its
max dimensions and then just multiply that with the number of joined
pipes. Any other joiner specific limits should be handled elsewhere.
Kandpal, Suraj Oct. 28, 2024, 4:13 a.m. UTC | #6
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Friday, October 25, 2024 1:35 PM
> To: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Cc: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org;
> intel-gfx@lists.freedesktop.org; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: Re: [PATCH 1/5] drm/i915/display: Fix the plane max height and width
> limits
> 
> On Fri, Oct 25, 2024 at 12:54:45PM +0530, Nautiyal, Ankit K wrote:
> >
> > On 10/25/2024 11:51 AM, Ville Syrjälä wrote:
> > > On Fri, Oct 25, 2024 at 11:31:32AM +0530, Suraj Kandpal wrote:
> > >> Fix the plane max height and width limits taking into account the
> > >> joined pipe limits too.
> > >>
> > >> Bspec: 28692, 49199, 68858
> > >> Fixes: 63dc014e37b9 ("drm/i915/dp: Allow big joiner modes in
> > >> intel_dp_mode_valid(), v3.")
> > >> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > >> ---
> > >>   drivers/gpu/drm/i915/display/intel_display.c | 19 ++++++++++++++++---
> > >>   1 file changed, 16 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > >> b/drivers/gpu/drm/i915/display/intel_display.c
> > >> index ef1436146325..fc578af4f394 100644
> > >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > >> @@ -8450,9 +8450,22 @@ intel_mode_valid_max_plane_size(struct
> drm_i915_private *dev_priv,
> > >>   	 * plane so let's not advertize modes that are
> > >>   	 * too big for that.
> > >>   	 */
> > >> -	if (DISPLAY_VER(dev_priv) >= 11) {
> > >> -		plane_width_max = 5120 * num_joined_pipes;
> > >> -		plane_height_max = 4320;
> > >> +	if (DISPLAY_VER(dev_priv) >= 20) {
> > >> +		if (num_joined_pipes > 1) {
> > >> +			plane_width_max = 8192;
> > >> +			plane_height_max = 4800;
> > >> +		} else {
> > >> +			plane_width_max = 5120;
> > >> +			plane_height_max = 4096;
> > > The joiner operates on horizontal lines. Why would the vertical
> > > resolution change here?
> > >
> > > And this is breaking ultrajoiner now.
> >
> > Yeah this will complicate function to check whether go to ultrajoiner
> > or bigjoiner.
> >
> > Perhaps need to have separate function for max_joined_plane_width()
> > and max_unjoined_plane_width();
> 
> I don't think max plane sizes have anything to with joiner. In fact we should
> probably make this code just ask the "primary" plane its max dimensions and
> then just multiply that with the number of joined pipes. Any other joiner
> specific limits should be handled elsewhere.

Then moving back to the old method of just having width as 5120*num_of_joined_pipe
And height as 4320 would be for the best

Regards,
Suraj Kandpal

> 
> --
> Ville Syrjälä
> Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ef1436146325..fc578af4f394 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8450,9 +8450,22 @@  intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
 	 * plane so let's not advertize modes that are
 	 * too big for that.
 	 */
-	if (DISPLAY_VER(dev_priv) >= 11) {
-		plane_width_max = 5120 * num_joined_pipes;
-		plane_height_max = 4320;
+	if (DISPLAY_VER(dev_priv) >= 20) {
+		if (num_joined_pipes > 1) {
+			plane_width_max = 8192;
+			plane_height_max = 4800;
+		} else {
+			plane_width_max = 5120;
+			plane_height_max = 4096;
+		}
+	} else if (DISPLAY_VER(dev_priv) >= 11) {
+		if (num_joined_pipes > 1) {
+			plane_width_max = 7680;
+			plane_height_max = 4320;
+		} else {
+			plane_width_max = 5120;
+			plane_height_max = 4096;
+		}
 	} else {
 		plane_width_max = 5120;
 		plane_height_max = 4096;