Weston / Wayland freeze

[robert_pasz]

Weston/Wayland issue related to DRM-KMS and display port. Weston version is 5.0.0, platform imx8m.

I have found an issue in libweston/compositor-drm.c. When connector is unplugged and plugged back, Weston sometimes do not attach primary plane (and get stuck). I found the place in the code where this problem is announced.

static bool
drm_plane_is_available(struct drm_plane *plane, struct drm_output *output)
{
    assert(plane->state_cur);

    /* The plane still has a request not yet completed by the kernel. */
    if (!plane->state_cur->complete) {
        weston_log("ROPA - Plane is bussy \n");
        return false;
    }
    /* The plane is still active on another output. */
    if (plane->state_cur->output && plane->state_cur->output != output) {
        weston_log("ROPA - Plane is still active on another output \n");
        return false;
    }
    /* Check whether the plane can be used with this CRTC; possible_crtcs
     * is a bitmask of CRTC indices (pipe), rather than CRTC object ID. */
    return !!(plane->possible_crtcs & (1 << output->pipe));
}

It looks that output is not freed when connector is unplugged. Does anybody know how to fix this issue? More precise the place in the code where could be legally released plane->state_cur->output?

Thanks

[Bio_TICFSL]

Hello Robert,

Using the Display Port Analyzer for generating the Hotplug event, I haven't been able to reproduce.

Question: Where do i find the log "ROPA - Plane is still active on another output" to confirm that I am reproducing your issue?

in  /var/volatile/log/weston.log?

Regards

[robert_pasz]

Hello,

Actually the code snippet comes from my local working copy. So the code

marked as ROPA is my comment.

Bug occurrence is quite random. Sometimes Weston have lost the connection

to actual plane.

We have using this version of Weston:

[Bio_TICFSL]

Hello,

What is your bsp version?  weston 5.0 seems very old version.

But, I feel this connection and disconnection is something like hotplug event handling.  You may add some debug message in compositor-drm.c to see what is going on when you plug and unplug your device, maybe this part of code is relevant,

udev_drm_event(int fd, uint32_t mask, void *data)

drm_backend_update_heads() 

drm_backend_update_unused_outputs()

to figure out what happened when connect and disconnect in good case, what sequence is missed in freeze state.

you can use weston debug, which will show you many useful info when app is running, this is how I did it.

killall weston
weston --tty=1 --debug

weston-debug drm-backend

start the application you want to debug for example weston-simple-egl in another terminal

Regards

[robert_pasz]

Hi,

I am using release our yocto sumo-4.14.98-2.0.0_ga  with weston 5.0.0. It is derived from yocto delivered with imx8m EVK kit. I have added driver for SN65DSI86 DSI to display port bridge. We have updated our board to support full DP (in spite that this circuit is primarily dedicated for eDP).

Good catch - I have observed display freeze at hot plug event. I have debugged that recompiling Weston with debug messages - as I have added some additional debug prints to the code to see what happened.

I have found some lost of pointer reference in function:

drm_output_destroy(struct weston_output *base) ..... // file ../libweston/compositor-drm.c

When output is released not all references to the output were released.

I have added that line

wl_list_for_each_safe(ps, next, &output->state_cur->plane_list, link) { if (ps->output == output) { ps->output = NULL; } }

before calling

drm_output_state_free(output->state_cur);

free(output);

That have solved this issue.

Regards,

[yyc1975]

Hi robert.pasz@congatec.com‌,

I have same HW and OS configuration as you and I am also trying to enable DP with SN65DSI86 bridge IC.

Could you share you driver to me? Thanks!

[robert_pasz]

Hi Yu-Yuan,

I am back in my office - just CORONA forced vacation - our department was forced to stay at home .......

So what about driver:

I have back-ported the driver from main line kernel 5.xx ++ (ti-sn65dsi86.c - drivers/gpu/drm/bridge/ti-sn65dsi86.c - Linux source code (v5.7.1) - Bootlin ). I am working now on hot plug event implementation - not fully done yet. I have changed also the function that set this bridge to ASSR mode - just to do not set it. TI supposed SN65DSI86 to be used with eDP panels. I am trying to use this chip in DP mode with standard monitors - it is necessary to disable ASSR (as most of DP displays doesn't support this mode). This is possible only if pin TEST2 of SN is connected to logic high (TI suggests to pull this pin to GND or left unconnected, but on the page 48 in the datasheet - inside description of register 0x5A bit ASSR_CONTROL it is mentioned to pull TEST2 pin to high - or it is not possible to disable ASSR otherwise).

More, I have implemented IRQ for HPD event - just to enable it and write GPIO triggered handler - there is code snippet that add HPD functionality (of course some data structures are updated):

static void ti_sn_hpd_work_handler(struct work_struct *work)
{
    struct ti_sn_bridge *pdata = work_to_ti_sn_bridge(work);
    u32 val = 0;

    regmap_read(pdata->regmap, SN_HPDLINE_REG, &val);
    /* update connector status */
    if (pdata->plugged != ((val & HPD_LINE_STATUS) != 0)) {
        /* changed */
        pdata->plugged = (val & HPD_LINE_STATUS);
        /* force enable or disable bridge - it seems that Wayland ignore
         * connector status .... */
        if (pdata->plugged) {
            /* plugged enable DP and train line */
            //ti_sn_bridge_enable(&pdata->bridge);
            ti_sn_enable_line(pdata);
        } else {
            /* unplugged disable DP */
            // ti_sn_bridge_disable(&pdata->bridge);
        }
    }

    ti_sn_dbg_connector_status(pdata, true);

    /* inform upper layers - will call connector detect */
    drm_helper_hpd_irq_event(pdata->connector.dev);

    /*re-enable IRQ */
    regmap_write(pdata->regmap, SN_IRQHPD_EN_REG, HPD_REMOVAL_IRQ_EN | HPD_INSERTION_IRQ_EN |
                 HPD_REPLUG_IRQ_EN);
    regmap_write(pdata->regmap, SN_IRQEN_REG, 0x01);
    enable_irq(pdata->hpd_irq);
}

/* IRQ HANDLER */
static irqreturn_t ti_sn_bridge_irq(int irq, void *dev_id)
{
    struct ti_sn_bridge *pdata = dev_id;
    unsigned int val = 0;
    int ret = 0;

    /* first disable IRQ */
    disable_irq_nosync(pdata->hpd_irq);

    /* read status register */
    ret = regmap_read(pdata->regmap, SN_IRQHPD_STATUS_REG, &val);
    regmap_write(pdata->regmap, SN_IRQHPD_STATUS_REG, val);

    /* Wake worker thread - do not stuck IRQ thread by long task */
    mod_delayed_work(system_wq, &pdata->mw, msecs_to_jiffies(300));

    return IRQ_HANDLED;
}

static void ti_sn_HPD_enable(struct ti_sn_bridge *pdata)
{
    int val = 0;

    /* enable HPD LINE */
    regmap_update_bits(pdata->regmap, SN_HPDLINE_REG, HPD_DISABLE, 0);
    /* wait for debounce */
    msleep(300);
    regmap_read(pdata->regmap, SN_HPDLINE_REG, &val);
    /* set connector status */
    pdata->plugged = ((val & HPD_LINE_STATUS) != 0);

    /* read IRQ status register */
    regmap_read(pdata->regmap, SN_IRQHPD_STATUS_REG, &val);
    /* if IRQ line is asserted - de-assert it */
    regmap_write(pdata->regmap, SN_IRQHPD_STATUS_REG, val);
    /*enable IRQ */
    regmap_write(pdata->regmap, SN_IRQHPD_EN_REG, HPD_REMOVAL_IRQ_EN | HPD_INSERTION_IRQ_EN |
                 HPD_REPLUG_IRQ_EN);
    regmap_write(pdata->regmap, SN_IRQEN_REG, 0x01);
    enable_irq(pdata->hpd_irq);
}

static void ti_sn_HPD_disable(struct ti_sn_bridge *pdata)
{
    int val = 0;

    /* read IRQ status register */
    regmap_read(pdata->regmap, SN_IRQHPD_STATUS_REG, &val);
    /* if IRQ line is asserted - de-assert it */
    regmap_write(pdata->regmap, SN_IRQHPD_STATUS_REG, val);
    /*disable IRQ */
    regmap_write(pdata->regmap, SN_IRQHPD_EN_REG, 0x00);
    regmap_write(pdata->regmap, SN_IRQEN_REG, 0x00);
    disable_irq(pdata->hpd_irq);

    /* fix data */
    pdata->plugged = false;
}

static int ti_sn_HPD_init(struct ti_sn_bridge *pdata,
                            struct i2c_client *client,
                            const struct i2c_device_id *id)
{
    int ret = 0;

    /* HPD IRQ */
    pdata->hpd_irq = client->irq;
    ret = devm_request_threaded_irq(pdata->dev, pdata->hpd_irq, NULL,
                        ti_sn_bridge_irq,
                        IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
                        dev_name(pdata->dev), pdata);
    if (ret < 0)
        return ret;
    disable_irq(pdata->hpd_irq);
    pdata->plugged = false;
    INIT_DELAYED_WORK(&pdata->mw, ti_sn_hpd_work_handler);

    return ret;
}

init is called from driver probe and enable/disable pair from bridge enable/disable. It didn't work at first - and then I found the issue with Weston.The fix discussed above make it work.

Yep another fix I did is in drivers/gpu/drm/bridge/sec-dsim.c. I have altered algorithm for clock setting in function  sec_mipi_dsim_check_pll_out() to achieve twice higher DSI frequency than it is originally calculated. It seems that SN bridge needs more time to synchronize -  if the DSI clock is set to the value originally calculated by NXP code the bridge is unable to synchronize !!!  (I just multiplied bit_clk by 2 before calling sec_mipi_dsim_calc_pmsk())

There is code snippet from sec-dsim.c:

int sec_mipi_dsim_check_pll_out(void *driver_private,
                const struct drm_display_mode *mode)
{   

...........................................

    dsim->pix_clk = pix_clk << 1; // multiply by TWO !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!1
    dsim->hpar = NULL;

    pmsk = sec_mipi_dsim_calc_pmsk(dsim);
    return 0;

...........................................
}
EXPORT_SYMBOL(sec_mipi_dsim_check_pll_out);

My driver is not final yet - another task is to read EDID from the display. Today I am using panel with DTB definition to set display mode now.

If you need additional infos do not hesitate to ask me -- now I will be faster in answer.

Best regards,

Robo P.