From: Maxime Ripard <mripard@kernel.org>
To: 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>,
"Andy Yan" <andy.yan@rock-chips.com>,
"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 11:45:33 +0200 [thread overview]
Message-ID: <20240516-lean-smooth-bonobo-d7e198@penduick> (raw)
In-Reply-To: <73944574.1631.18f6be1e78f.Coremail.andyshrk@163.com>
[-- Attachment #1: Type: text/plain, Size: 7259 bytes --]
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?
* 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 again,
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
next prev parent reply other threads:[~2024-05-16 9:45 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 ` Maxime Ripard [this message]
2024-05-16 10:33 ` [PATCH " Andy Yan
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=20240516-lean-smooth-bonobo-d7e198@penduick \
--to=mripard@kernel.org \
--cc=airlied@gmail.com \
--cc=andy.yan@rock-chips.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=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).