[PATCH v13 15/28] drm/connector: hdmi: Compute bpc and format automatically

Andy Yan andy.yan at rock-chips.com
Thu May 16 03:33:21 PDT 2024


Hi Maxime,

On 5/16/24 17:45, Maxime Ripard wrote:
> Hi again,
> 
> On Sun, May 12, 2024 at 04:18:38PM +0800, Andy Yan wrote:
>> 在 2024-05-07 21:17:33,"Maxime Ripard" <mripard at kernel.org> 写道:
>>> Now that we have all the infrastructure needed, we can add some code
>>> that will, for a given connector state and mode, compute the best output
>>> format and bpc.
>>>
>>> The algorithm is equivalent to the one already found in i915 and vc4.
>>>
>>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>> Signed-off-by: Maxime Ripard <mripard at kernel.org>
>>> ---
>>> drivers/gpu/drm/display/drm_hdmi_state_helper.c    | 199 ++++++++++++++++++++-
>>> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c |  25 ++-
>>> 2 files changed, 212 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>>> index 063421835dba..f20dcfecb6b8 100644
>>> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>>> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
>>> @@ -1,9 +1,11 @@
>>> // SPDX-License-Identifier: MIT
>>>
>>> #include <drm/drm_atomic.h>
>>> #include <drm/drm_connector.h>
>>> +#include <drm/drm_edid.h>
>>> +#include <drm/drm_print.h>
>>>
>>> #include <drm/display/drm_hdmi_helper.h>
>>> #include <drm/display/drm_hdmi_state_helper.h>
>>>
>>> /**
>>> @@ -46,10 +48,112 @@ connector_state_get_mode(const struct drm_connector_state *conn_state)
>>> 		return NULL;
>>>
>>> 	return &crtc_state->mode;
>>> }
>>>
>>> +static bool
>>> +sink_supports_format_bpc(const struct drm_connector *connector,
>>> +			 const struct drm_display_info *info,
>>> +			 const struct drm_display_mode *mode,
>>> +			 unsigned int format, unsigned int bpc)
>>> +{
>>> +	struct drm_device *dev = connector->dev;
>>> +	u8 vic = drm_match_cea_mode(mode);
>>> +
>>> +	/*
>>> +	 * CTA-861-F, section 5.4 - Color Coding & Quantization states
>>> +	 * that the bpc must be 8, 10, 12 or 16 except for the default
>>> +	 * 640x480 VIC1 where the value must be 8.
>>> +	 *
>>> +	 * The definition of default here is ambiguous but the spec
>>> +	 * refers to VIC1 being the default timing in several occasions
>>> +	 * so our understanding is that for the default timing (ie,
>>> +	 * VIC1), the bpc must be 8.
>>> +	 */
>>> +	if (vic == 1 && bpc != 8) {
>>> +		drm_dbg_kms(dev, "VIC1 requires a bpc of 8, got %u\n", bpc);
>>> +		return false;
>>> +	}
>>> +
>>> +	if (!info->is_hdmi &&
>>> +	    (format != HDMI_COLORSPACE_RGB || bpc != 8)) {
>>> +		drm_dbg_kms(dev, "DVI Monitors require an RGB output at 8 bpc\n");
>>> +		return false;
>>> +	}
>>> +
>>> +	if (!(connector->hdmi.supported_formats & BIT(format))) {
>>> +		drm_dbg_kms(dev, "%s format unsupported by the connector.\n",
>>> +			    drm_hdmi_connector_get_output_format_name(format));
>>> +		return false;
>>> +	}
>>> +
>>> +	switch (format) {
>>> +	case HDMI_COLORSPACE_RGB:
>>> +		drm_dbg_kms(dev, "RGB Format, checking the constraints.\n");
>>> +
>>> +		if (!(info->color_formats & DRM_COLOR_FORMAT_RGB444)) {
>>> +			drm_dbg_kms(dev, "Sink doesn't support RGB.\n");
>>> +			return false;
>>> +		}
>>> +
>> As I reported in V12,  the HDMI output on my rk3036-kylin was lost after apply this series.
>> This is because there is something wrong with the DDC on my board, the edid read always failed
>> on first bootup. That means inno_hdmi_connector_get_modes will return 0.
>>
>> and in function drm_helper_probe_single_connector_modes:
>>
>>           count = drm_helper_probe_get_modes(connector);
>>
>>           if (count == 0 && (connector->status == connector_status_connected ||
>>                              connector->status == connector_status_unknown)) {
>>                   count = drm_add_modes_noedid(connector, 1024, 768);
>>
>>                   /*
>>                    * Section 4.2.2.6 (EDID Corruption Detection) of the DP 1.4a
>>                    * Link CTS specifies that 640x480 (the official "failsafe"
>>                    * mode) needs to be the default if there's no EDID.
>>                    */
>>                   if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
>>                           drm_set_preferred_mode(connector, 640, 480);
>>           }
>> drm_add_modes_noedid will not initialize display_info. So the check about display info will always failed here:
>>
>> [    4.205368] rockchip-drm display-subsystem: [drm:drm_atomic_check_only] checking (ptrval)
>> [    4.205410] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CRTC:35:crtc-0] mode changed
>> [    4.205439] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CRTC:35:crtc-0] enable changed
>> [    4.205464] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CRTC:35:crtc-0] active changed
>> [    4.205490] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] Updating routing for [CONNECTOR:37:HDMI-A-1]
>> [    4.205517] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CONNECTOR:37:HDMI-A-1] using [ENCODER:36:TMDS-36] on [CRTC:35:crtc-0]
>> [    4.205545] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] Trying with a 8 bpc output
>> [    4.205575] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] Trying RGB output format
>> [    4.205670] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] RGB Format, checking the constraints.
>> [    4.205696] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] Sink doesn't support RGB.
>> [    4.205720] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] RGB output format not supported with 8 bpc
>> [    4.205747] rockchip-drm display-subsystem: [drm:drm_atomic_helper_connector_hdmi_check] Failed. No Format Supported for that bpc count.
>> [    4.205772] rockchip-drm display-subsystem: [drm:drm_atomic_helper_check_modeset] [CONNECTOR:37:HDMI-A-1] driver check failed
>> [    4.205796] rockchip-drm display-subsystem: [drm:drm_atomic_check_only] atomic driver check for (ptrval) failed: -22
>>
>> My reply for your email in V12[0] was bounced, so I think you didn't read it.
>>
>> [0]https://patchwork.kernel.org/project/linux-rockchip/patch/20240423-kms-hdmi-connector-state-v12-27-3338e4c0b189@kernel.org/
> 
> Indeed, I never received it, sorry.
> 
> Thanks for looking into it, it's very valuable.
> 
> I can see several things that interact and could go wrong:
> 
> * The DDC readout should not fail like that. From a quick look at the
>    driver, I'm wondering if it's not due to the fact that the DDC
>    controller isn't powered until the first modeset happens. Since the
>    first get_modes call is done with the controller disabled, it's
>    probably not initialized enough yet. The first modeset then comes and
>    will initialize the controller enough for the subsequent get_modes to
>    work. Is it something you could look into?

I have a same feeling about it. I will check it later.
> 
> * drm_display_info not being filled to some sane default when there's no
>    EDID is indeed an issue. I can't be made generic, but the HDMI spec
>    provides us with some minimum requirements we can probably set in this
>    case (RGB supported, 8bpc supported, etc.) I'll work on that.

Thanks.
> 
> Thanks again,
> Maxime



More information about the linux-arm-kernel mailing list