[PATCH v2 09/12] drm/i915/display: Factor out intel_display_{min, max}_pipe_bpp()

[Nautiyal, Ankit K]

On 5/6/2025 8:46 PM, Imre Deak wrote:
> On Tue, May 06, 2025 at 06:37:50PM +0530, Nautiyal, Ankit K wrote:
>> On 4/28/2025 7:01 PM, Imre Deak wrote:
>>> Factor out helpers that can be used in a follow-up change to query the
>>> minimum and maximum pipe bpp supported by the HW.
>>>
>>> Signed-off-by: Imre Deak <imre.deak at intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/display/intel_display.c | 28 +++++++++++++-------
>>>    drivers/gpu/drm/i915/display/intel_display.h |  3 +++
>>>    drivers/gpu/drm/i915/display/intel_dp.c      |  2 +-
>>>    3 files changed, 22 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>> index b4ddffe53e23f..cf2c11826ffb3 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -4301,6 +4301,22 @@ compute_sink_pipe_bpp(const struct drm_connector_state *conn_state,
>>>    	return 0;
>>>    }
>>> +int intel_display_min_pipe_bpp(void)
>>> +{
>>> +	return 6 * 3;
>> Hmm.. I think this is DP specific, for HDMI min is 8 * 3 = 24 bpp.
>>
>> I see this is function is used while checking for min bpp while adding
>> support for forcing a bpp for different connectors.
>>
>> Would it make sense to make this connector specific?
> I wonder if it could be kept simple for now and use the platform's min
> bpp value in the above debugfs entry. IIUC the
> platform/connector/output_type specific min pipe bpps are:
>
> DP/RGB:      18
> DP/YCBCR:    24
> LVDS:        18
> DSI/DDI:     24
> DSI/non-DDI: 18
> All other:   24
>
> It would make sense to add a helper and use it everywhere, but it would
> be a bigger change. Are you ok to do this as a follow up?

Yes we can have a follow up for this, to get this connector specific.

Should we make the min as 24 for now, that will work for most things.

Or else we can let that be 18 for now, and the tests when forcing a 
particular bpp, will then use modeset with try commit.

If commit fails with -EINVAL, tests will try again with higher bpp.


Regards,

Ankit



>
>> Regards,
>>
>> Ankit
>>
>>
>>> +}
>>> +
>>> +int intel_display_max_pipe_bpp(struct intel_display *display)
>>> +{
>>> +	if (display->platform.g4x || display->platform.valleyview ||
>>> +	    display->platform.cherryview)
>>> +		return 10*3;
>>> +	else if (DISPLAY_VER(display) >= 5)
>>> +		return 12*3;
>>> +	else
>>> +		return 8*3;
>>> +}
>>> +
>>>    static int
>>>    compute_baseline_pipe_bpp(struct intel_atomic_state *state,
>>>    			  struct intel_crtc *crtc)
>>> @@ -4310,17 +4326,9 @@ compute_baseline_pipe_bpp(struct intel_atomic_state *state,
>>>    		intel_atomic_get_new_crtc_state(state, crtc);
>>>    	struct drm_connector *connector;
>>>    	struct drm_connector_state *connector_state;
>>> -	int bpp, i;
>>> +	int i;
>>> -	if (display->platform.g4x || display->platform.valleyview ||
>>> -	    display->platform.cherryview)
>>> -		bpp = 10*3;
>>> -	else if (DISPLAY_VER(display) >= 5)
>>> -		bpp = 12*3;
>>> -	else
>>> -		bpp = 8*3;
>>> -
>>> -	crtc_state->pipe_bpp = bpp;
>>> +	crtc_state->pipe_bpp = intel_display_max_pipe_bpp(display);
>>>    	/* Clamp display bpp to connector max bpp */
>>>    	for_each_new_connector_in_state(&state->base, connector, connector_state, i) {
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
>>> index 3b54a62c290af..b6610e9175a7a 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.h
>>> @@ -524,6 +524,9 @@ void intel_plane_fixup_bitmasks(struct intel_crtc_state *crtc_state);
>>>    bool intel_crtc_vrr_disabling(struct intel_atomic_state *state,
>>>    			      struct intel_crtc *crtc);
>>> +int intel_display_min_pipe_bpp(void);
>>> +int intel_display_max_pipe_bpp(struct intel_display *display);
>>> +
>>>    /* modesetting */
>>>    int intel_modeset_pipes_in_mask_early(struct intel_atomic_state *state,
>>>    				      const char *reason, u8 pipe_mask);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index 0f89a301e4a0d..73ca9f8efefc5 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -1198,7 +1198,7 @@ intel_dp_output_format(struct intel_connector *connector,
>>>    int intel_dp_min_bpp(enum intel_output_format output_format)
>>>    {
>>>    	if (output_format == INTEL_OUTPUT_FORMAT_RGB)
>>> -		return 6 * 3;
>>> +		return intel_display_min_pipe_bpp();
>>>    	else
>>>    		return 8 * 3;
>>>    }