From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from canpmsgout06.his.huawei.com (canpmsgout06.his.huawei.com [113.46.200.221]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C7C11426ED6 for ; Tue, 28 Apr 2026 12:53:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=113.46.200.221 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777380840; cv=none; b=q4fURYi0vxDI9yej6DHyH3P+NdBWQWbe6l+W0iR9SHUN3C3kj0f6Gy+191+lDSkGYjWbHJvKjs+2/iSvgC2V0qE88h3TBVqRGyGzKqe/6jcHcrlfkbbxdlm7PkYSAr9B5OZhi7HOr2IiER+UQRtXYPNEJ56sA5a2xwYJSCG3djA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777380840; c=relaxed/simple; bh=MoLYtWnXMbRiUqnsIbM/3sePsPqzxARyHvsn6dbgL0E=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=iXEGpp55X+kZqmOnikJ53LoGGTUlhomozHN1NNVOEG4IVuDNPBlT/ohxzkrx9Wa3p1CNyCUq7bXDgd59/WijpUL9gDDTUN92nNSONQro8RbpGtmN1Osp6w8z/PMgMYw9bhLGbbVisaPDvG2nqGGPEZltj2UIBJS8S7vhWpb8nes= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b=w+iFWUnT; arc=none smtp.client-ip=113.46.200.221 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b="w+iFWUnT" dkim-signature: v=1; a=rsa-sha256; d=huawei.com; s=dkim; c=relaxed/relaxed; q=dns/txt; h=From; bh=g0UHuHSmppTHkK8ZnXb61KDzIovDlgrgK0GvCMgNYnI=; b=w+iFWUnTb33XqWO9nldGLfnTwbJRSggBYvyanldRops6aiwKIZUX8ixbrewtuKUcKnjD25lvx vowjXRRwk6v4EWKvzYXVbiwT5YuasCmfAXU3pPEWa6aZ0PCZnedXZYWK5YAYrjeix4HEZjRmXBW qlT8ZNqNM4ovnO8BFUwaBO4= Received: from mail.maildlp.com (unknown [172.19.162.140]) by canpmsgout06.his.huawei.com (SkyGuard) with ESMTPS id 4g4gF61MzhzRhQx; Tue, 28 Apr 2026 20:47:22 +0800 (CST) Received: from dggemv712-chm.china.huawei.com (unknown [10.1.198.32]) by mail.maildlp.com (Postfix) with ESMTPS id 848A420333; Tue, 28 Apr 2026 20:53:48 +0800 (CST) Received: from kwepemq100007.china.huawei.com (7.202.195.175) by dggemv712-chm.china.huawei.com (10.1.198.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Tue, 28 Apr 2026 20:53:48 +0800 Received: from [10.159.167.44] (10.159.167.44) by kwepemq100007.china.huawei.com (7.202.195.175) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Tue, 28 Apr 2026 20:53:47 +0800 Message-ID: Date: Tue, 28 Apr 2026 20:53:47 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH for drm-misc-fixes v5 2/4] drm/hisilicon/hibmc: fix no showing when no connectors connected To: Thomas Zimmermann , , , , , , , , CC: , , , , , , , , References: <20260423063233.1267631-1-shiyongbang@huawei.com> <20260423063233.1267631-3-shiyongbang@huawei.com> <27864583-4211-4553-bdc0-42dadd25d212@suse.de> <057ebc30-f103-4d24-b5be-bbc9b79050e8@suse.de> From: Yongbang Shi In-Reply-To: <057ebc30-f103-4d24-b5be-bbc9b79050e8@suse.de> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-ClientProxiedBy: kwepems100001.china.huawei.com (7.221.188.238) To kwepemq100007.china.huawei.com (7.202.195.175) > Hi > > Am 28.04.26 um 05:58 schrieb Yongbang Shi: > [...] >>> There's a problem with the overall logic here: if the physical status is 'connected' and the helper could not retrieve >>> any modes, the helper should return 0 here. Then the DRM helpers do the right thing with setting up a few modes or an >>> EDID override as a fallback. See [2]. For example, on my broken test system, I'd be able to provide my display's EDID >>> and get the correct output. >>> >>> [2] https://elixir.bootlin.com/linux/v7.0.1/source/drivers/gpu/drm/drm_probe_helper.c#L436 >>> >>> Any code below is BMC setup and should run in an else branch, just like in ast. >>> >> The `drm_edid_override_connector_update` interface is used to retrieve an overridden EDID from debugfs or from firmware: >>    * Debugfs: Requires the user to manually perform file operations to configure it; >>    * Firmware: Requires the distribution’s operating system or the user to place the EDID binary file in the >>                `/lib/firmware/` directory, and the `drm.edid_firmware` parameter must be specified in the GRUB boot >>                parameters; >> >> The modification method you provided allows for display even when the EDID cannot be retrieved. But both of these >> methods require cooperation from the user or the distribution's operating system, making implementation relatively >> difficult. > > Yes, it's the established way for users to override the EDID. > >> >> Our use case requires that the KVM's display functionality remain intact even when no VGA or DisplayPort monitor is >> connected. I believe setting `drm_set_preferred_mode` (1024x768) as the default resolution when no monitor is present >> would be a more universal solution. Similarly, on your test system, this approach would ensure basic display >> functionality. > > If no modes could be detected on the VGA and no EDID has been provided by the user, DRM helpers will install a set of > default modes that are suitable for graphics cards. See [1]. The modes installed for the KVM are not all compatible with > VGA.  If you rely on DRM helpers, you'll always get correct modes for VGA and KVM. > > Generally speaking, the decision of VGA-vs-KVM should be in ->detect. Having phys_state/phys_status does this very well. > The ->get_modes function should then use whatever has been detected. > > [1] https://elixir.bootlin.com/linux/v7.0.1/source/drivers/gpu/drm/drm_probe_helper.c#L653 > Yes, I understand what you mean. But there's another issue here; we've also tested the scenario where we directly return 0. However, we found that only three resolutions remain supported: 1024*768, 800*600 and 640*480. This is because the resolution added via `drm_add_modes_noedid` in the helper has a maximum of 1024*768. This doesn't look good—there are too few resolutions available for users to choose from in KVM. With the current implementation, when `count == 0`, we set the resolution to the highest resolution supported by our CRTC, allowing users more options. Of course, this implementation has its drawbacks; it doesn't allow users to provide a custom resolution list via debugfs using the override method when `count == 0`. But this method isn't very important for how our product is used. Thanks, Yongbang. > Best regards > Thomas > >> >>>>    +    drm_edid_connector_update(connector, NULL); >>>>        count = drm_add_modes_noedid(connector, >>>>                         connector->dev->mode_config.max_width, >>>>                         connector->dev->mode_config.max_height); >>>>        drm_set_preferred_mode(connector, 1024, 768); >>>>    -out: >>>> -    drm_edid_free(drm_edid); >>>> - >>>>        return count; >>>>    } >>>>    @@ -57,10 +50,32 @@ static void hibmc_connector_destroy(struct drm_connector *connector) >>>>        drm_connector_cleanup(connector); >>>>    } >>>>    +static int hibmc_vdac_detect(struct drm_connector *connector, >>>> +                 struct drm_modeset_acquire_ctx *ctx, >>>> +                 bool force) >>>> +{ >>>> +    struct hibmc_drm_private *priv = to_hibmc_drm_private(connector->dev); >>>> +    int state = drm_connector_helper_detect_from_ddc(connector, ctx, >>>> +                             force); >>> 'state' -> 'status' >>> >> Okay. >> >>>> +    struct hibmc_vdac *vdac = to_hibmc_vdac(connector); >>>> + >>>> +    if (priv->dp.phys_state == connector_status_connected) >>>> +        return vdac->phys_state = state; >>> Please only one statement per line. First assign, then return. >>> >> Yes, Sorry about that. According to the Linux kernel coding guidelines, this line of code should be split. >> >> >> Thanks, >> Yongbang. >> >>>> + >>>> +    if (state != vdac->phys_state) >>>> +        ++connector->epoch_counter; >>>> +    vdac->phys_state = state; >>>> + >>>> +    /* If both the DP and VDAC physical states are disconnected, >>>> +     * the "connected" status is returned to support KVM display. >>>> +     */ >>>> +    return connector_status_connected; >>> I haven't tried, but I think this should also resolve the problems on my test systems. Great, thanks a lot! I might just >>> get default resolutions for now, but that's OK. >>> >>> Best regards >>> Thomas >>> >>>> +} >>>> + >>>>    static const struct drm_connector_helper_funcs >>>>        hibmc_connector_helper_funcs = { >>>>        .get_modes = hibmc_connector_get_modes, >>>> -    .detect_ctx = drm_connector_helper_detect_from_ddc, >>>> +    .detect_ctx = hibmc_vdac_detect, >>>>    }; >>>>      static const struct drm_connector_funcs hibmc_connector_funcs = { >>>> @@ -130,6 +145,8 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv) >>>>          connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT; >>>>    +    vdac->phys_state = connector_status_connected; >>>> + >>>>        return 0; >>>>      err: >