From: Andy Yan <andy.yan@rock-chips.com>
To: Maxime Ripard <mripard@kernel.org>, Andy Yan <andyshrk@163.com>
Cc: "Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"David Airlie" <airlied@gmail.com>,
"Daniel Vetter" <daniel@ffwll.ch>,
"Jonathan Corbet" <corbet@lwn.net>,
"Sandy Huang" <hjc@rock-chips.com>,
"Heiko Stübner" <heiko@sntech.de>, "Chen-Yu Tsai" <wens@csie.org>,
"Jernej Skrabec" <jernej.skrabec@gmail.com>,
"Samuel Holland" <samuel@sholland.org>,
"Hans Verkuil" <hverkuil@xs4all.nl>,
"Sebastian Wick" <sebastian.wick@redhat.com>,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
dri-devel@lists.freedesktop.org,
linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
linux-rockchip@lists.infradead.org, linux-sunxi@lists.linux.dev
Subject: Re: [PATCH v13 15/28] drm/connector: hdmi: Compute bpc and format automatically
Date: Thu, 16 May 2024 18:33:21 +0800 [thread overview]
Message-ID: <d0757dd6-ec00-407b-af53-f60f34cf0580@rock-chips.com> (raw)
In-Reply-To: <20240516-lean-smooth-bonobo-d7e198@penduick>
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@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@linux.intel.com>
>>> Signed-off-by: Maxime Ripard <mripard@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
next prev parent reply other threads:[~2024-05-16 10:44 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-07 13:17 [PATCH v13 00/28] drm/connector: Create HDMI Connector infrastructure Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 01/28] drm/connector: Introduce an HDMI connector initialization function Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 02/28] drm/mode_object: Export drm_mode_obj_find_prop_id for tests Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 03/28] drm/tests: connector: Add tests for drmm_connector_hdmi_init Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 04/28] drm/connector: hdmi: Create an HDMI sub-state Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 05/28] drm/connector: hdmi: Add output BPC to the connector state Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 06/28] drm/tests: Add output bpc tests Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 07/28] drm/connector: hdmi: Add support for output format Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 08/28] drm/tests: Add output formats tests Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 09/28] drm/display: hdmi: Add HDMI compute clock helper Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 10/28] drm/tests: Add HDMI TDMS character rate tests Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 11/28] drm/connector: hdmi: Calculate TMDS character rate Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 12/28] drm/tests: Add TDMS character rate connector state tests Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 13/28] drm/connector: hdmi: Add custom hook to filter TMDS character rate Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 14/28] drm/tests: Add HDMI connector rate filter hook tests Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 15/28] drm/connector: hdmi: Compute bpc and format automatically Maxime Ripard
2024-05-12 8:18 ` Andy Yan
2024-05-16 9:45 ` [PATCH " Maxime Ripard
2024-05-16 10:33 ` Andy Yan [this message]
2024-05-07 13:17 ` [PATCH v13 16/28] drm/tests: Add HDMI connector bpc and format tests Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 17/28] drm/connector: hdmi: Add Broadcast RGB property Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 18/28] drm/tests: Add tests for " Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 19/28] drm/connector: hdmi: Add RGB Quantization Range to the connector state Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 20/28] drm/tests: Add RGB Quantization tests Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 21/28] drm/connector: hdmi: Add Infoframes generation Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 22/28] drm/tests: Add infoframes test Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 23/28] drm/connector: hdmi: Create Infoframe DebugFS entries Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 24/28] drm/vc4: hdmi: Switch to HDMI connector Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 25/28] drm/vc4: tests: Remove vc4_dummy_plane structure Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 26/28] drm/vc4: tests: Convert to plane creation helper Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 27/28] drm/rockchip: inno_hdmi: Switch to HDMI connector Maxime Ripard
2024-05-12 8:29 ` Andy Yan
2024-05-16 9:36 ` [PATCH " Maxime Ripard
2024-05-07 13:17 ` [PATCH v13 28/28] drm/sun4i: hdmi: " Maxime Ripard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d0757dd6-ec00-407b-af53-f60f34cf0580@rock-chips.com \
--to=andy.yan@rock-chips.com \
--cc=airlied@gmail.com \
--cc=andyshrk@163.com \
--cc=corbet@lwn.net \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=heiko@sntech.de \
--cc=hjc@rock-chips.com \
--cc=hverkuil@xs4all.nl \
--cc=jernej.skrabec@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=samuel@sholland.org \
--cc=sebastian.wick@redhat.com \
--cc=tzimmermann@suse.de \
--cc=ville.syrjala@linux.intel.com \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).