Re: [PATCH for drm-misc-fixes v4 2/4] drm/hisilicon/hibmc: fix no showing when no connectors connected
From: Yongbang Shi
Date: Tue Apr 21 2026 - 03:52:56 EST
> Hi
>
> Am 21.04.26 um 09:24 schrieb Yongbang Shi:
> [...]
>>> That's not really much of a problem. Just filter the older boards by some board identifier and handle it separately. We
>>> use that all over the kernel; including graphics drives. See for example DMI_MATCH. [1]
>>>
>> Okay, we'll need to verify this with the BIOS and do some analysis. If necessary, we'll submit a patch as a feature
>> later on.
>
> Right. It's a separate thing.
>
>
>>> I think the design in the current patch is sound. It just needs to increment epoch_count if the physical status changes;
>>> and it would trigger these user-space bugs. But we can only work around the latter.
>>>
>> Okay, I understand, but there's one more thing I need to confirm: simply adding logic to increment `epoch_count` within
>> `vdac_detect` isn't enough, is it? We still need to enable the `connector.polled` mechanism for periodic detection,
>> right?
>
> Exactly. The poll helper will periodically run the detect helper to test if the display is still attached. This is
> required on connectors that do not auto-detect this, such as VGA.
>
> The VGA standard assumes that the display would be connected while the computer was running. But everyone always
> connected and disconnected monitors at runtime, so we have to support that.
>
Okay, this will further improve our hibmc-drm driver. The v5 patches are coming soon.
Thanks,
Yongbang.
> Best regards
> Thomas
>
>>
>> like this:
>> hibmc_kms_init()
>> {
>> ...
>>
>> drm_kms_helper_poll_init(dev);
>>
>> ...
>> }
>>
>> a little change like this:
>> vdac_detect()
>> {
>> phys_state = detect_from_hw()
>>
>> if (dp.phys_state == connected) {
>> vdac->phys_state = phys_state;
>> return vdac->phys_state; /* return from here, epoch_counter will be update by drm framework */
>> }
>>
>> if (phys_state != vdac->phys_state)
>> ++epoch_counter;
>>
>> vdac->phys_state = phys_state;
>>
>> return connector_status_connected;
>> }
>>
>> Thanks,
>> Yongbang.
>>
>>> Best regards
>>> Thomas
>>>
>>>
>>>>>> Over the next few days, we will focus on testing relevant scenarios
>>>>>> while setting both the VGA and DP ports to `connected` to see if we
>>>>>> encounter similar issues or warnings.
>>>>>>
>>>>>>> You also need to increment the connector's epoch_counter if the physical
>>>>>>> status changed. Doing this will trigger DRM clients to re-read the
>>>>>>> connector state and reconfigure display.
>>>>>>>
>>>>>>> Something like that
>>>>>>>
>>>>>>> dp_detect()
>>>>>>> {
>>>>>>> dp.phys_state = detect_from_hw()
>>>>>>>
>>>>>>> if (dp.phys_state changed)
>>>>>>> ++epoch_counter
>>>>>>> return connected
>>>>>>> }
>>>>>>>
>>>>>>> vdac_detect()
>>>>>>> {
>>>>>>> if (dp.phys_state == connected)
>>>>>>> return disconnected
>>>>>>>
>>>>>> Only one option is displayed here, but it does not meet our specifications.
>>>>>>
>>>>>>> vdac.phys_state = detect_from_hw()
>>>>>>>
>>>>>>> if (vdac.phys_state changed)
>>>>>>> ++epoch_counter
>>>>>>> return connected
>>>>>>> }
>>>>>>>
>>>>>>> This handles the KVM in each connector's helper. The ast driver is
>>>>>>> probably the best reference for the current logic. See [2].
>>>>>>>
>>>>>> I looked at the implementations of `detect_ctx` in ATS and MGAG200;
>>>>>> these implementations generally always return `connected`.
>>>>>>
>>>>>> If my analysis is correct, the logic should be as follows:
>>>>>> - If the `detect_ctx` function implemented by each vendor returns the
>>>>>> actual hardware state, there is no need to modify `epoch_counter`;
>>>>>> the DRM framework will execute `epoch_counter++` based on the return
>>>>>> value of the `detect_ctx` function;
>>>>>>
>>>>>> - If the `detect_ctx` function in a vendor’s code always returns
>>>>>> `connected`, then `epoch_counter++` must be performed within the
>>>>>> vendor’s `detect_ctx` function; the implementations in ATS and
>>>>>> MGAG200 appear to be like this;
>>>>> Exactly. In ast/mgag200, we had the case that the VGA got disconnected. We'd still return connected, as the BMC/KVM
>>>>> was
>>>>> present. We now had to reconfigure for the KVM. That requires modifying epoch_counter. You can see at [1] that we only
>>>>> increment if the physical status changes. [1] Same goes for re-connecting the VGA display.
>>>>>
>>>>> [1] https://elixir.bootlin.com/linux/v7.0/source/drivers/gpu/drm/ast/ast_vga.c#L56
>>>>>
>>>>>> Here is the pseudocode for our current logic. This logic is indeed a bit
>>>>>> hard to grasp, so I've added some more comments; it might be a bit
>>>>>> clearer than the patch:
>>>>>>
>>>>>> dp_detect()
>>>>>> {
>>>>>> dp.phys_state = detect_from_hw()
>>>>>> /* epoch_counter update by drm framework */
>>>>>> return dp.phys_state;
>>>>>> }
>>>>>>
>>>>>> vdac_detect()
>>>>>> {
>>>>>> /*
>>>>>> * Scene 1: The driver runs on an older version of the BMC chip,
>>>>>> * and `dp.phys_state` is initialized to 0. To maintain
>>>>>> * consistency with the previous hibmc-drm driver
>>>>>> * logic, always return `connected`.
>>>>>> * Scene 2: The driver runs on an new version of the BMC chip,
>>>>>> * and `dp.phys_state` is real status, witch is real
>>>>>> * `disconnect`. In this scenario, we must ensure that
>>>>>> * the VGA is `connected` to guarantee that the KVM
>>>>>> * displays properly.
>>>>>> */
>>>>>> if (dp.phys_state != connected)
>>>>>> return connected;
>>>>>>
>>>>>> /*
>>>>>> * DP is connected: This driver is definitely running on the
>>>>>> * latest BMC chip, and the DDC I2C is definitely
>>>>>> * connected to the VGA port, so we can confirm whether
>>>>>> * the VGA is present by reading the EDID directly via
>>>>>> * I2C.
>>>>>> */
>>>>>> vdac.phys_state = detect_from_hw()
>>>>>> /* epoch_counter update by drm framework */
>>>>>> return vdac.phys_state;
>>>>> Let's assume the DP is disconnected. If VGA's physical status goes from connected to disconnected, user space will not
>>>>> know that it needs to reconfigure for the BMC. (or vice versa)
>>>>>
>>>>> To give you some context from earlier discussions around the other drivers: We had the scenario that the user boots up
>>>>> without a connected VGA display. This would enable the BMC, which sometimes picks a large resolution; say >2k per
>>>>> dimension. That is problematic for KVM over networks. And if the user now plugs in a physical VGA display, they
>>>>> would be
>>>>> stuck with the large resolution on the framebuffer console. Maybe the VGA display wouldn't even support such a
>>>>> high-res
>>>>> mode. So we set the default for the BMC to 1024x768. That is well manageable for all of today's VGA displays and does
>>>>> not impose too much overhead for KVM over networks. As a drawback, it's a bit small on monitors and not nice to look
>>>>> at. So we modify epoch_counter whenever the physical status changes. This triggers compositors to reconfigure their
>>>>> screens for the new display.
>>>>>
>>>> Yes, this issue does exist. If we uses the `connector.polled` mechanism to monitor the VGA's presence in real time and
>>>> update the `epoch_counter`, which might resolve the issue. However, upon further consideration, I don't think this is
>>>> the optimal solution. If we create a separate connector for KVM and distinguish between older-generation BMC chips and
>>>> newer-generation BMC chips, the problem will be easily solved. In addition, DP and VGA, as well as VGA and KVM, will be
>>>> decoupled.
>>>>
>>>> A temporary solution:
>>>> /* called by poll work */
>>>> vdac_detect()
>>>> {
>>>> /* Let’s examine this logic on its own to gain a better understanding. */
>>>> if (older-generation)
>>>> return connected;
>>>>
>>>> vdac.phys_state = detect_from_hw();
>>>> if (vdac.phys_state changed)
>>>> ++epoch_counter;
>>>>
>>>> if (dp.phys_state != connected) {
>>>> return connected;
>>>> }
>>>>
>>>> return vdac.phys_state;
>>>> }
>>>>
>>>>
>>>> A more elegant solution:
>>>> |--connector0(KVM): always connected, do not update epoch_counter.
>>>> |
>>>> crtc--|--connector1(VGA): detect real time by `connector.polled`, update epoch_counter.
>>>> |
>>>> |--connector1(DP): detect real time by hpd interrupt, update epoch_counter.
>>>>
>>>> kvm_detect()
>>>> {
>>>> return connected;
>>>> }
>>>>
>>>> /* called by poll work */
>>>> vdac_detect()
>>>> {
>>>> /* for older-generation */
>>>> if (older-generation)
>>>> return connected;
>>>>
>>>> /* for newer-generation */
>>>> vdac.phys_state = detect_from_hw();
>>>>
>>>> /* epoch_counter update by drm framework */
>>>> return vdac.phys_state;
>>>> }
>>>>
>>>> dp_detect()
>>>> {
>>>> dp.phys_state = detect_from_hw();
>>>>
>>>> /* epoch_counter update by drm framework */
>>>> return dp.phys_state;
>>>> }
>>>>
>>>> Thanks,
>>>> Yongbang.
>>>>
>>>>> Best regards
>>>>> Thomas
>>>>>
>>>>>> }
>>>>>>
>>>>>> Sorry for the delay; it took me a little while to figure out how best to
>>>>>> explain this bug and our current implementation.
>>>>>>
>>>>>> Thanks,
>>>>>> Yongbang.
>>>>>>
>>>>>>> [1] https://gitlab.gnome.org/GNOME/mutter/-/issues/3858
>>>>>>> [2] https://elixir.bootlin.com/linux/v7.0/source/drivers/gpu/drm/ast/
>>>>>>> ast_vga.c#L47
>>>>>>>
>>>>>>> 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:
>