| Message ID | 20201105120333.947408-26-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert DSI code to use drm_mipi_dsi and drm_panel | expand |
Hi Tomi and Sebastian, Thank you for the patch. On Thu, Nov 05, 2020 at 02:03:02PM +0200, Tomi Valkeinen wrote: > From: Sebastian Reichel <sebastian.reichel@collabora.com> > > In preparation for removing custom DSS calls from the DSI > panel driver, this moves support for external tearing event > GPIOs into the DSI host driver. This way tearing events are > always handled in the core resulting in simplification of > the panel drivers. > > The TE GPIO acquisition follows works in the same way as the > exynos DSI implementation. > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > .../gpu/drm/omapdrm/displays/panel-dsi-cm.c | 101 +------------ > drivers/gpu/drm/omapdrm/dss/dsi.c | 138 ++++++++++++++++-- > 2 files changed, 133 insertions(+), 106 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > index 8890ee2ba830..43f63b5a120b 100644 > --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c > @@ -54,7 +54,6 @@ struct panel_drv_data { > > /* panel HW configuration from DT or platform data */ > struct gpio_desc *reset_gpio; > - struct gpio_desc *ext_te_gpio; > > struct regulator_bulk_data supplies[DCS_REGULATOR_SUPPLY_NUM]; > > @@ -68,10 +67,6 @@ struct panel_drv_data { > > bool te_enabled; > > - atomic_t do_update; > - > - struct delayed_work te_timeout_work; > - > bool intro_printed; > > struct workqueue_struct *workqueue; > @@ -83,8 +78,6 @@ struct panel_drv_data { > > #define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev) > > -static irqreturn_t dsicm_te_isr(int irq, void *data); > -static void dsicm_te_timeout_work_callback(struct work_struct *work); > static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable); > > static int dsicm_panel_reset(struct panel_drv_data *ddata); > @@ -240,9 +233,6 @@ static int dsicm_enter_ulps(struct panel_drv_data *ddata) > if (r) > goto err; > > - if (ddata->ext_te_gpio) > - disable_irq(gpiod_to_irq(ddata->ext_te_gpio)); > - > src->ops->dsi.disable(src, false, true); > > ddata->ulps_enabled = true; > @@ -271,15 +261,12 @@ static int dsicm_exit_ulps(struct panel_drv_data *ddata) > src->ops->enable(src); > ddata->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; > > - r = _dsicm_enable_te(ddata, true); > + r = _dsicm_enable_te(ddata, ddata->te_enabled); > if (r) { > dev_err(&ddata->dsi->dev, "failed to re-enable TE"); > goto err2; > } > > - if (ddata->ext_te_gpio) > - enable_irq(gpiod_to_irq(ddata->ext_te_gpio)); > - > dsicm_queue_ulps_work(ddata); > > ddata->ulps_enabled = false; > @@ -290,11 +277,8 @@ static int dsicm_exit_ulps(struct panel_drv_data *ddata) > dev_err(&ddata->dsi->dev, "failed to exit ULPS"); > > r = dsicm_panel_reset(ddata); > - if (!r) { > - if (ddata->ext_te_gpio) > - enable_irq(gpiod_to_irq(ddata->ext_te_gpio)); > + if (!r) > ddata->ulps_enabled = false; > - } > > dsicm_queue_ulps_work(ddata); > > @@ -745,43 +729,6 @@ static void dsicm_framedone_cb(int err, void *data) > src->ops->dsi.bus_unlock(src); > } > > -static irqreturn_t dsicm_te_isr(int irq, void *data) > -{ > - struct panel_drv_data *ddata = data; > - struct omap_dss_device *src = ddata->src; > - int old; > - int r; > - > - old = atomic_cmpxchg(&ddata->do_update, 1, 0); > - > - if (old) { > - cancel_delayed_work(&ddata->te_timeout_work); > - > - r = src->ops->dsi.update(src, ddata->dsi->channel, dsicm_framedone_cb, > - ddata); > - if (r) > - goto err; > - } > - > - return IRQ_HANDLED; > -err: > - dev_err(&ddata->dsi->dev, "start update failed\n"); > - src->ops->dsi.bus_unlock(src); > - return IRQ_HANDLED; > -} > - > -static void dsicm_te_timeout_work_callback(struct work_struct *work) > -{ > - struct panel_drv_data *ddata = container_of(work, struct panel_drv_data, > - te_timeout_work.work); > - struct omap_dss_device *src = ddata->src; > - > - dev_err(&ddata->dsi->dev, "TE not received for 250ms!\n"); > - > - atomic_set(&ddata->do_update, 0); > - src->ops->dsi.bus_unlock(src); > -} > - > static int dsicm_update(struct omap_dss_device *dssdev, > u16 x, u16 y, u16 w, u16 h) > { > @@ -809,16 +756,10 @@ static int dsicm_update(struct omap_dss_device *dssdev, > if (r) > goto err; > > - if (ddata->te_enabled && ddata->ext_te_gpio) { > - schedule_delayed_work(&ddata->te_timeout_work, > - msecs_to_jiffies(250)); > - atomic_set(&ddata->do_update, 1); > - } else { > - r = src->ops->dsi.update(src, ddata->dsi->channel, dsicm_framedone_cb, > - ddata); > - if (r) > - goto err; > - } > + r = src->ops->dsi.update(src, ddata->dsi->channel, dsicm_framedone_cb, > + ddata); > + if (r) > + goto err; > > /* note: no bus_unlock here. unlock is src framedone_cb */ > mutex_unlock(&ddata->lock); > @@ -840,8 +781,7 @@ static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable) > else > r = mipi_dsi_dcs_set_tear_off(dsi); > > - if (!ddata->ext_te_gpio) > - src->ops->dsi.enable_te(src, enable); > + src->ops->dsi.enable_te(src, enable); > > /* possible panel bug */ > msleep(100); > @@ -934,14 +874,6 @@ static int dsicm_probe_of(struct mipi_dsi_device *dsi) > return err; > } > > - ddata->ext_te_gpio = devm_gpiod_get_optional(&dsi->dev, "te", > - GPIOD_IN); > - if (IS_ERR(ddata->ext_te_gpio)) { > - err = PTR_ERR(ddata->ext_te_gpio); > - dev_err(&dsi->dev, "TE gpio request failed: %d", err); > - return err; > - } > - > err = of_get_display_timing(node, "panel-timing", &timing); > if (!err) { > videomode_from_timing(&timing, &ddata->vm); > @@ -1024,25 +956,6 @@ static int dsicm_probe(struct mipi_dsi_device *dsi) > > mutex_init(&ddata->lock); > > - atomic_set(&ddata->do_update, 0); > - > - if (ddata->ext_te_gpio) { > - r = devm_request_irq(dev, gpiod_to_irq(ddata->ext_te_gpio), > - dsicm_te_isr, > - IRQF_TRIGGER_RISING, > - "taal vsync", ddata); > - > - if (r) { > - dev_err(dev, "IRQ request failed\n"); > - goto err_reg; > - } > - > - INIT_DEFERRABLE_WORK(&ddata->te_timeout_work, > - dsicm_te_timeout_work_callback); > - > - dev_dbg(dev, "Using GPIO TE\n"); > - } > - > ddata->workqueue = create_singlethread_workqueue("dsicm_wq"); > if (!ddata->workqueue) { > r = -ENOMEM; > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 003d26cead5a..921e7a1e1014 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -14,7 +14,9 @@ > #include <linux/device.h> > #include <linux/err.h> > #include <linux/interrupt.h> > +#include <linux/irq.h> > #include <linux/delay.h> > +#include <linux/gpio/consumer.h> You could sort the headers while at it :-) > #include <linux/mutex.h> > #include <linux/module.h> > #include <linux/semaphore.h> > @@ -368,6 +370,11 @@ struct dsi_data { > unsigned int update_bytes; > #endif > > + /* external TE GPIO */ > + struct gpio_desc *te_gpio; > + struct delayed_work te_timeout_work; > + atomic_t do_ext_te_update; > + > bool te_enabled; > bool ulps_enabled; > > @@ -3827,19 +3834,12 @@ static void dsi_framedone_irq_callback(void *data) > dsi_handle_framedone(dsi, 0); > } > > -static int dsi_update(struct omap_dss_device *dssdev, int channel, > - void (*callback)(int, void *), void *data) > +static int _dsi_update(struct dsi_data *dsi) > { > - struct dsi_data *dsi = to_dsi_data(dssdev); > u16 dw, dh; > > dsi_perf_mark_setup(dsi); > > - dsi->update_channel = channel; > - > - dsi->framedone_callback = callback; > - dsi->framedone_data = data; > - > dw = dsi->vm.hactive; > dh = dsi->vm.vactive; > > @@ -3852,6 +3852,26 @@ static int dsi_update(struct omap_dss_device *dssdev, int channel, > return 0; > } > > +static int dsi_update(struct omap_dss_device *dssdev, int channel, > + void (*callback)(int, void *), void *data) > +{ > + struct dsi_data *dsi = to_dsi_data(dssdev); > + > + dsi->update_channel = channel; > + dsi->framedone_callback = callback; > + dsi->framedone_data = data; > + > + if (dsi->te_enabled && dsi->te_gpio) { > + schedule_delayed_work(&dsi->te_timeout_work, > + msecs_to_jiffies(250)); > + atomic_set(&dsi->do_ext_te_update, 1); > + } else { > + _dsi_update(dsi); > + } > + > + return 0; > +} > + > /* Display funcs */ > > static int dsi_configure_dispc_clocks(struct dsi_data *dsi) > @@ -4095,6 +4115,14 @@ static int dsi_enable_te(struct omap_dss_device *dssdev, bool enable) > struct dsi_data *dsi = to_dsi_data(dssdev); > > dsi->te_enabled = enable; > + > + if (dsi->te_gpio) { > + if (enable) > + enable_irq(gpiod_to_irq(dsi->te_gpio)); > + else > + disable_irq(gpiod_to_irq(dsi->te_gpio)); > + } > + > return 0; > } > > @@ -4772,11 +4800,89 @@ static const struct omap_dss_device_ops dsi_ops = { > }, > }; > > +static irqreturn_t omap_dsi_te_irq_handler(int irq, void *dev_id) > +{ > + struct dsi_data *dsi = (struct dsi_data *)dev_id; > + int old; > + > + old = atomic_cmpxchg(&dsi->do_ext_te_update, 1, 0); > + if (old) { > + cancel_delayed_work(&dsi->te_timeout_work); > + _dsi_update(dsi); > + } > + > + return IRQ_HANDLED; > +} > + > +static void omap_dsi_te_timeout_work_callback(struct work_struct *work) > +{ > + struct dsi_data *dsi = container_of(work, struct dsi_data, > + te_timeout_work.work); Strange indentation. > + int old; > + > + old = atomic_cmpxchg(&dsi->do_ext_te_update, 1, 0); > + if (old) { > + dev_err(dsi->dev, "TE not received for 250ms!\n"); > + _dsi_update(dsi); > + } > +} > + > +static int omap_dsi_register_te_irq(struct dsi_data *dsi, > + struct mipi_dsi_device *client) > +{ > + int err; > + int te_irq; > + > + dsi->te_gpio = gpiod_get_from_of_node(client->dev.of_node, Could we call gpiod_get(&client->dev, "te", ...) instead ? > + "te-gpios", 0, GPIOD_IN, > + "dsi-tearing-effect"); > + if (IS_ERR(dsi->te_gpio)) { > + err = PTR_ERR(dsi->te_gpio); > + > + if (err == -ENOENT) { > + dsi->te_gpio = NULL; > + return 0; > + } > + > + dev_err(dsi->dev, "Could not get TE gpio: %d\n", err); > + return err; > + } > + > + te_irq = gpiod_to_irq(dsi->te_gpio); Could this fail ? > + irq_set_status_flags(te_irq, IRQ_NOAUTOEN); > + > + err = request_threaded_irq(te_irq, omap_dsi_te_irq_handler, NULL, > + IRQF_TRIGGER_RISING, "TE", dsi); What's the reason to request a threaded IRQ instead of a regular IRQ if the threaded handler is NULL ? > + if (err) { > + dev_err(dsi->dev, "request irq failed with %d\n", err); > + gpiod_put(dsi->te_gpio); > + return err; > + } > + > + INIT_DEFERRABLE_WORK(&dsi->te_timeout_work, > + omap_dsi_te_timeout_work_callback); > + > + dev_dbg(dsi->dev, "Using GPIO TE\n"); > + > + return 0; > +} > + > +static void omap_dsi_unregister_te_irq(struct dsi_data *dsi) > +{ > + if (dsi->te_gpio) { > + free_irq(gpiod_to_irq(dsi->te_gpio), dsi); You could store the IRQ number in dsi_data to avoid recomputing it. > + cancel_delayed_work(&dsi->te_timeout_work); > + gpiod_put(dsi->te_gpio); > + dsi->te_gpio = NULL; > + } > +} > + > static int omap_dsi_host_attach(struct mipi_dsi_host *host, > struct mipi_dsi_device *client) > { > struct dsi_data *dsi = host_to_omap(host); > unsigned int channel = client->channel; > + int r; > > if (channel > 3) > return -EINVAL; > @@ -4791,13 +4897,20 @@ static int omap_dsi_host_attach(struct mipi_dsi_host *host, > return -EINVAL; > } > > - dsi->vc[channel].dest = client; > + atomic_set(&dsi->do_ext_te_update, 0); > > - dsi->pix_fmt = client->format; > - if (client->mode_flags & MIPI_DSI_MODE_VIDEO) > + if (client->mode_flags & MIPI_DSI_MODE_VIDEO) { > dsi->mode = OMAP_DSS_DSI_VIDEO_MODE; > - else > + } else { > + r = omap_dsi_register_te_irq(dsi, client); > + if (r) > + return r; > + > dsi->mode = OMAP_DSS_DSI_CMD_MODE; > + } > + > + dsi->vc[channel].dest = client; > + dsi->pix_fmt = client->format; > > return 0; > } > @@ -4814,6 +4927,7 @@ static int omap_dsi_host_detach(struct mipi_dsi_host *host, > if (dsi->vc[channel].dest != client) > return -EINVAL; > > + omap_dsi_unregister_te_irq(dsi); > dsi->vc[channel].dest = NULL; > return 0; > }
On 09/11/2020 11:19, Laurent Pinchart wrote: > Hi Tomi and Sebastian, > > Thank you for the patch. > > On Thu, Nov 05, 2020 at 02:03:02PM +0200, Tomi Valkeinen wrote: >> From: Sebastian Reichel <sebastian.reichel@collabora.com> >> >> In preparation for removing custom DSS calls from the DSI >> panel driver, this moves support for external tearing event >> GPIOs into the DSI host driver. This way tearing events are >> always handled in the core resulting in simplification of >> the panel drivers. >> >> The TE GPIO acquisition follows works in the same way as the >> exynos DSI implementation. >> >> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> .../gpu/drm/omapdrm/displays/panel-dsi-cm.c | 101 +------------ >> drivers/gpu/drm/omapdrm/dss/dsi.c | 138 ++++++++++++++++-- >> 2 files changed, 133 insertions(+), 106 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c >> index 8890ee2ba830..43f63b5a120b 100644 >> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c >> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c >> @@ -54,7 +54,6 @@ struct panel_drv_data { >> >> /* panel HW configuration from DT or platform data */ >> struct gpio_desc *reset_gpio; >> - struct gpio_desc *ext_te_gpio; >> >> struct regulator_bulk_data supplies[DCS_REGULATOR_SUPPLY_NUM]; >> >> @@ -68,10 +67,6 @@ struct panel_drv_data { >> >> bool te_enabled; >> >> - atomic_t do_update; >> - >> - struct delayed_work te_timeout_work; >> - >> bool intro_printed; >> >> struct workqueue_struct *workqueue; >> @@ -83,8 +78,6 @@ struct panel_drv_data { >> >> #define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev) >> >> -static irqreturn_t dsicm_te_isr(int irq, void *data); >> -static void dsicm_te_timeout_work_callback(struct work_struct *work); >> static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable); >> >> static int dsicm_panel_reset(struct panel_drv_data *ddata); >> @@ -240,9 +233,6 @@ static int dsicm_enter_ulps(struct panel_drv_data *ddata) >> if (r) >> goto err; >> >> - if (ddata->ext_te_gpio) >> - disable_irq(gpiod_to_irq(ddata->ext_te_gpio)); >> - >> src->ops->dsi.disable(src, false, true); >> >> ddata->ulps_enabled = true; >> @@ -271,15 +261,12 @@ static int dsicm_exit_ulps(struct panel_drv_data *ddata) >> src->ops->enable(src); >> ddata->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; >> >> - r = _dsicm_enable_te(ddata, true); >> + r = _dsicm_enable_te(ddata, ddata->te_enabled); >> if (r) { >> dev_err(&ddata->dsi->dev, "failed to re-enable TE"); >> goto err2; >> } >> >> - if (ddata->ext_te_gpio) >> - enable_irq(gpiod_to_irq(ddata->ext_te_gpio)); >> - >> dsicm_queue_ulps_work(ddata); >> >> ddata->ulps_enabled = false; >> @@ -290,11 +277,8 @@ static int dsicm_exit_ulps(struct panel_drv_data *ddata) >> dev_err(&ddata->dsi->dev, "failed to exit ULPS"); >> >> r = dsicm_panel_reset(ddata); >> - if (!r) { >> - if (ddata->ext_te_gpio) >> - enable_irq(gpiod_to_irq(ddata->ext_te_gpio)); >> + if (!r) >> ddata->ulps_enabled = false; >> - } >> >> dsicm_queue_ulps_work(ddata); >> >> @@ -745,43 +729,6 @@ static void dsicm_framedone_cb(int err, void *data) >> src->ops->dsi.bus_unlock(src); >> } >> >> -static irqreturn_t dsicm_te_isr(int irq, void *data) >> -{ >> - struct panel_drv_data *ddata = data; >> - struct omap_dss_device *src = ddata->src; >> - int old; >> - int r; >> - >> - old = atomic_cmpxchg(&ddata->do_update, 1, 0); >> - >> - if (old) { >> - cancel_delayed_work(&ddata->te_timeout_work); >> - >> - r = src->ops->dsi.update(src, ddata->dsi->channel, dsicm_framedone_cb, >> - ddata); >> - if (r) >> - goto err; >> - } >> - >> - return IRQ_HANDLED; >> -err: >> - dev_err(&ddata->dsi->dev, "start update failed\n"); >> - src->ops->dsi.bus_unlock(src); >> - return IRQ_HANDLED; >> -} >> - >> -static void dsicm_te_timeout_work_callback(struct work_struct *work) >> -{ >> - struct panel_drv_data *ddata = container_of(work, struct panel_drv_data, >> - te_timeout_work.work); >> - struct omap_dss_device *src = ddata->src; >> - >> - dev_err(&ddata->dsi->dev, "TE not received for 250ms!\n"); >> - >> - atomic_set(&ddata->do_update, 0); >> - src->ops->dsi.bus_unlock(src); >> -} >> - >> static int dsicm_update(struct omap_dss_device *dssdev, >> u16 x, u16 y, u16 w, u16 h) >> { >> @@ -809,16 +756,10 @@ static int dsicm_update(struct omap_dss_device *dssdev, >> if (r) >> goto err; >> >> - if (ddata->te_enabled && ddata->ext_te_gpio) { >> - schedule_delayed_work(&ddata->te_timeout_work, >> - msecs_to_jiffies(250)); >> - atomic_set(&ddata->do_update, 1); >> - } else { >> - r = src->ops->dsi.update(src, ddata->dsi->channel, dsicm_framedone_cb, >> - ddata); >> - if (r) >> - goto err; >> - } >> + r = src->ops->dsi.update(src, ddata->dsi->channel, dsicm_framedone_cb, >> + ddata); >> + if (r) >> + goto err; >> >> /* note: no bus_unlock here. unlock is src framedone_cb */ >> mutex_unlock(&ddata->lock); >> @@ -840,8 +781,7 @@ static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable) >> else >> r = mipi_dsi_dcs_set_tear_off(dsi); >> >> - if (!ddata->ext_te_gpio) >> - src->ops->dsi.enable_te(src, enable); >> + src->ops->dsi.enable_te(src, enable); >> >> /* possible panel bug */ >> msleep(100); >> @@ -934,14 +874,6 @@ static int dsicm_probe_of(struct mipi_dsi_device *dsi) >> return err; >> } >> >> - ddata->ext_te_gpio = devm_gpiod_get_optional(&dsi->dev, "te", >> - GPIOD_IN); >> - if (IS_ERR(ddata->ext_te_gpio)) { >> - err = PTR_ERR(ddata->ext_te_gpio); >> - dev_err(&dsi->dev, "TE gpio request failed: %d", err); >> - return err; >> - } >> - >> err = of_get_display_timing(node, "panel-timing", &timing); >> if (!err) { >> videomode_from_timing(&timing, &ddata->vm); >> @@ -1024,25 +956,6 @@ static int dsicm_probe(struct mipi_dsi_device *dsi) >> >> mutex_init(&ddata->lock); >> >> - atomic_set(&ddata->do_update, 0); >> - >> - if (ddata->ext_te_gpio) { >> - r = devm_request_irq(dev, gpiod_to_irq(ddata->ext_te_gpio), >> - dsicm_te_isr, >> - IRQF_TRIGGER_RISING, >> - "taal vsync", ddata); >> - >> - if (r) { >> - dev_err(dev, "IRQ request failed\n"); >> - goto err_reg; >> - } >> - >> - INIT_DEFERRABLE_WORK(&ddata->te_timeout_work, >> - dsicm_te_timeout_work_callback); >> - >> - dev_dbg(dev, "Using GPIO TE\n"); >> - } >> - >> ddata->workqueue = create_singlethread_workqueue("dsicm_wq"); >> if (!ddata->workqueue) { >> r = -ENOMEM; >> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c >> index 003d26cead5a..921e7a1e1014 100644 >> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c >> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c >> @@ -14,7 +14,9 @@ >> #include <linux/device.h> >> #include <linux/err.h> >> #include <linux/interrupt.h> >> +#include <linux/irq.h> >> #include <linux/delay.h> >> +#include <linux/gpio/consumer.h> > > You could sort the headers while at it :-) I'll do that after the series. >> #include <linux/mutex.h> >> #include <linux/module.h> >> #include <linux/semaphore.h> >> @@ -368,6 +370,11 @@ struct dsi_data { >> unsigned int update_bytes; >> #endif >> >> + /* external TE GPIO */ >> + struct gpio_desc *te_gpio; >> + struct delayed_work te_timeout_work; >> + atomic_t do_ext_te_update; >> + >> bool te_enabled; >> bool ulps_enabled; >> >> @@ -3827,19 +3834,12 @@ static void dsi_framedone_irq_callback(void *data) >> dsi_handle_framedone(dsi, 0); >> } >> >> -static int dsi_update(struct omap_dss_device *dssdev, int channel, >> - void (*callback)(int, void *), void *data) >> +static int _dsi_update(struct dsi_data *dsi) >> { >> - struct dsi_data *dsi = to_dsi_data(dssdev); >> u16 dw, dh; >> >> dsi_perf_mark_setup(dsi); >> >> - dsi->update_channel = channel; >> - >> - dsi->framedone_callback = callback; >> - dsi->framedone_data = data; >> - >> dw = dsi->vm.hactive; >> dh = dsi->vm.vactive; >> >> @@ -3852,6 +3852,26 @@ static int dsi_update(struct omap_dss_device *dssdev, int channel, >> return 0; >> } >> >> +static int dsi_update(struct omap_dss_device *dssdev, int channel, >> + void (*callback)(int, void *), void *data) >> +{ >> + struct dsi_data *dsi = to_dsi_data(dssdev); >> + >> + dsi->update_channel = channel; >> + dsi->framedone_callback = callback; >> + dsi->framedone_data = data; >> + >> + if (dsi->te_enabled && dsi->te_gpio) { >> + schedule_delayed_work(&dsi->te_timeout_work, >> + msecs_to_jiffies(250)); >> + atomic_set(&dsi->do_ext_te_update, 1); >> + } else { >> + _dsi_update(dsi); >> + } >> + >> + return 0; >> +} >> + >> /* Display funcs */ >> >> static int dsi_configure_dispc_clocks(struct dsi_data *dsi) >> @@ -4095,6 +4115,14 @@ static int dsi_enable_te(struct omap_dss_device *dssdev, bool enable) >> struct dsi_data *dsi = to_dsi_data(dssdev); >> >> dsi->te_enabled = enable; >> + >> + if (dsi->te_gpio) { >> + if (enable) >> + enable_irq(gpiod_to_irq(dsi->te_gpio)); >> + else >> + disable_irq(gpiod_to_irq(dsi->te_gpio)); >> + } >> + >> return 0; >> } >> >> @@ -4772,11 +4800,89 @@ static const struct omap_dss_device_ops dsi_ops = { >> }, >> }; >> >> +static irqreturn_t omap_dsi_te_irq_handler(int irq, void *dev_id) >> +{ >> + struct dsi_data *dsi = (struct dsi_data *)dev_id; >> + int old; >> + >> + old = atomic_cmpxchg(&dsi->do_ext_te_update, 1, 0); >> + if (old) { >> + cancel_delayed_work(&dsi->te_timeout_work); >> + _dsi_update(dsi); >> + } >> + >> + return IRQ_HANDLED; >> +} >> + >> +static void omap_dsi_te_timeout_work_callback(struct work_struct *work) >> +{ >> + struct dsi_data *dsi = container_of(work, struct dsi_data, >> + te_timeout_work.work); > > Strange indentation. > >> + int old; >> + >> + old = atomic_cmpxchg(&dsi->do_ext_te_update, 1, 0); >> + if (old) { >> + dev_err(dsi->dev, "TE not received for 250ms!\n"); >> + _dsi_update(dsi); >> + } >> +} >> + >> +static int omap_dsi_register_te_irq(struct dsi_data *dsi, >> + struct mipi_dsi_device *client) >> +{ >> + int err; >> + int te_irq; >> + >> + dsi->te_gpio = gpiod_get_from_of_node(client->dev.of_node, > > Could we call gpiod_get(&client->dev, "te", ...) instead ? Yep. >> + "te-gpios", 0, GPIOD_IN, >> + "dsi-tearing-effect"); >> + if (IS_ERR(dsi->te_gpio)) { >> + err = PTR_ERR(dsi->te_gpio); >> + >> + if (err == -ENOENT) { >> + dsi->te_gpio = NULL; >> + return 0; >> + } >> + >> + dev_err(dsi->dev, "Could not get TE gpio: %d\n", err); >> + return err; >> + } >> + >> + te_irq = gpiod_to_irq(dsi->te_gpio); > > Could this fail ? Yes, I'll add error handling. >> + irq_set_status_flags(te_irq, IRQ_NOAUTOEN); >> + >> + err = request_threaded_irq(te_irq, omap_dsi_te_irq_handler, NULL, >> + IRQF_TRIGGER_RISING, "TE", dsi); > > What's the reason to request a threaded IRQ instead of a regular IRQ if > the threaded handler is NULL ? Probably just a mistake. I'll change this to use the threaded handler. >> + if (err) { >> + dev_err(dsi->dev, "request irq failed with %d\n", err); >> + gpiod_put(dsi->te_gpio); >> + return err; >> + } >> + >> + INIT_DEFERRABLE_WORK(&dsi->te_timeout_work, >> + omap_dsi_te_timeout_work_callback); >> + >> + dev_dbg(dsi->dev, "Using GPIO TE\n"); >> + >> + return 0; >> +} >> + >> +static void omap_dsi_unregister_te_irq(struct dsi_data *dsi) >> +{ >> + if (dsi->te_gpio) { >> + free_irq(gpiod_to_irq(dsi->te_gpio), dsi); > > You could store the IRQ number in dsi_data to avoid recomputing it. Yep. Tomi
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c index 8890ee2ba830..43f63b5a120b 100644 --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c @@ -54,7 +54,6 @@ struct panel_drv_data { /* panel HW configuration from DT or platform data */ struct gpio_desc *reset_gpio; - struct gpio_desc *ext_te_gpio; struct regulator_bulk_data supplies[DCS_REGULATOR_SUPPLY_NUM]; @@ -68,10 +67,6 @@ struct panel_drv_data { bool te_enabled; - atomic_t do_update; - - struct delayed_work te_timeout_work; - bool intro_printed; struct workqueue_struct *workqueue; @@ -83,8 +78,6 @@ struct panel_drv_data { #define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev) -static irqreturn_t dsicm_te_isr(int irq, void *data); -static void dsicm_te_timeout_work_callback(struct work_struct *work); static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable); static int dsicm_panel_reset(struct panel_drv_data *ddata); @@ -240,9 +233,6 @@ static int dsicm_enter_ulps(struct panel_drv_data *ddata) if (r) goto err; - if (ddata->ext_te_gpio) - disable_irq(gpiod_to_irq(ddata->ext_te_gpio)); - src->ops->dsi.disable(src, false, true); ddata->ulps_enabled = true; @@ -271,15 +261,12 @@ static int dsicm_exit_ulps(struct panel_drv_data *ddata) src->ops->enable(src); ddata->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; - r = _dsicm_enable_te(ddata, true); + r = _dsicm_enable_te(ddata, ddata->te_enabled); if (r) { dev_err(&ddata->dsi->dev, "failed to re-enable TE"); goto err2; } - if (ddata->ext_te_gpio) - enable_irq(gpiod_to_irq(ddata->ext_te_gpio)); - dsicm_queue_ulps_work(ddata); ddata->ulps_enabled = false; @@ -290,11 +277,8 @@ static int dsicm_exit_ulps(struct panel_drv_data *ddata) dev_err(&ddata->dsi->dev, "failed to exit ULPS"); r = dsicm_panel_reset(ddata); - if (!r) { - if (ddata->ext_te_gpio) - enable_irq(gpiod_to_irq(ddata->ext_te_gpio)); + if (!r) ddata->ulps_enabled = false; - } dsicm_queue_ulps_work(ddata); @@ -745,43 +729,6 @@ static void dsicm_framedone_cb(int err, void *data) src->ops->dsi.bus_unlock(src); } -static irqreturn_t dsicm_te_isr(int irq, void *data) -{ - struct panel_drv_data *ddata = data; - struct omap_dss_device *src = ddata->src; - int old; - int r; - - old = atomic_cmpxchg(&ddata->do_update, 1, 0); - - if (old) { - cancel_delayed_work(&ddata->te_timeout_work); - - r = src->ops->dsi.update(src, ddata->dsi->channel, dsicm_framedone_cb, - ddata); - if (r) - goto err; - } - - return IRQ_HANDLED; -err: - dev_err(&ddata->dsi->dev, "start update failed\n"); - src->ops->dsi.bus_unlock(src); - return IRQ_HANDLED; -} - -static void dsicm_te_timeout_work_callback(struct work_struct *work) -{ - struct panel_drv_data *ddata = container_of(work, struct panel_drv_data, - te_timeout_work.work); - struct omap_dss_device *src = ddata->src; - - dev_err(&ddata->dsi->dev, "TE not received for 250ms!\n"); - - atomic_set(&ddata->do_update, 0); - src->ops->dsi.bus_unlock(src); -} - static int dsicm_update(struct omap_dss_device *dssdev, u16 x, u16 y, u16 w, u16 h) { @@ -809,16 +756,10 @@ static int dsicm_update(struct omap_dss_device *dssdev, if (r) goto err; - if (ddata->te_enabled && ddata->ext_te_gpio) { - schedule_delayed_work(&ddata->te_timeout_work, - msecs_to_jiffies(250)); - atomic_set(&ddata->do_update, 1); - } else { - r = src->ops->dsi.update(src, ddata->dsi->channel, dsicm_framedone_cb, - ddata); - if (r) - goto err; - } + r = src->ops->dsi.update(src, ddata->dsi->channel, dsicm_framedone_cb, + ddata); + if (r) + goto err; /* note: no bus_unlock here. unlock is src framedone_cb */ mutex_unlock(&ddata->lock); @@ -840,8 +781,7 @@ static int _dsicm_enable_te(struct panel_drv_data *ddata, bool enable) else r = mipi_dsi_dcs_set_tear_off(dsi); - if (!ddata->ext_te_gpio) - src->ops->dsi.enable_te(src, enable); + src->ops->dsi.enable_te(src, enable); /* possible panel bug */ msleep(100); @@ -934,14 +874,6 @@ static int dsicm_probe_of(struct mipi_dsi_device *dsi) return err; } - ddata->ext_te_gpio = devm_gpiod_get_optional(&dsi->dev, "te", - GPIOD_IN); - if (IS_ERR(ddata->ext_te_gpio)) { - err = PTR_ERR(ddata->ext_te_gpio); - dev_err(&dsi->dev, "TE gpio request failed: %d", err); - return err; - } - err = of_get_display_timing(node, "panel-timing", &timing); if (!err) { videomode_from_timing(&timing, &ddata->vm); @@ -1024,25 +956,6 @@ static int dsicm_probe(struct mipi_dsi_device *dsi) mutex_init(&ddata->lock); - atomic_set(&ddata->do_update, 0); - - if (ddata->ext_te_gpio) { - r = devm_request_irq(dev, gpiod_to_irq(ddata->ext_te_gpio), - dsicm_te_isr, - IRQF_TRIGGER_RISING, - "taal vsync", ddata); - - if (r) { - dev_err(dev, "IRQ request failed\n"); - goto err_reg; - } - - INIT_DEFERRABLE_WORK(&ddata->te_timeout_work, - dsicm_te_timeout_work_callback); - - dev_dbg(dev, "Using GPIO TE\n"); - } - ddata->workqueue = create_singlethread_workqueue("dsicm_wq"); if (!ddata->workqueue) { r = -ENOMEM; diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c index 003d26cead5a..921e7a1e1014 100644 --- a/drivers/gpu/drm/omapdrm/dss/dsi.c +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c @@ -14,7 +14,9 @@ #include <linux/device.h> #include <linux/err.h> #include <linux/interrupt.h> +#include <linux/irq.h> #include <linux/delay.h> +#include <linux/gpio/consumer.h> #include <linux/mutex.h> #include <linux/module.h> #include <linux/semaphore.h> @@ -368,6 +370,11 @@ struct dsi_data { unsigned int update_bytes; #endif + /* external TE GPIO */ + struct gpio_desc *te_gpio; + struct delayed_work te_timeout_work; + atomic_t do_ext_te_update; + bool te_enabled; bool ulps_enabled; @@ -3827,19 +3834,12 @@ static void dsi_framedone_irq_callback(void *data) dsi_handle_framedone(dsi, 0); } -static int dsi_update(struct omap_dss_device *dssdev, int channel, - void (*callback)(int, void *), void *data) +static int _dsi_update(struct dsi_data *dsi) { - struct dsi_data *dsi = to_dsi_data(dssdev); u16 dw, dh; dsi_perf_mark_setup(dsi); - dsi->update_channel = channel; - - dsi->framedone_callback = callback; - dsi->framedone_data = data; - dw = dsi->vm.hactive; dh = dsi->vm.vactive; @@ -3852,6 +3852,26 @@ static int dsi_update(struct omap_dss_device *dssdev, int channel, return 0; } +static int dsi_update(struct omap_dss_device *dssdev, int channel, + void (*callback)(int, void *), void *data) +{ + struct dsi_data *dsi = to_dsi_data(dssdev); + + dsi->update_channel = channel; + dsi->framedone_callback = callback; + dsi->framedone_data = data; + + if (dsi->te_enabled && dsi->te_gpio) { + schedule_delayed_work(&dsi->te_timeout_work, + msecs_to_jiffies(250)); + atomic_set(&dsi->do_ext_te_update, 1); + } else { + _dsi_update(dsi); + } + + return 0; +} + /* Display funcs */ static int dsi_configure_dispc_clocks(struct dsi_data *dsi) @@ -4095,6 +4115,14 @@ static int dsi_enable_te(struct omap_dss_device *dssdev, bool enable) struct dsi_data *dsi = to_dsi_data(dssdev); dsi->te_enabled = enable; + + if (dsi->te_gpio) { + if (enable) + enable_irq(gpiod_to_irq(dsi->te_gpio)); + else + disable_irq(gpiod_to_irq(dsi->te_gpio)); + } + return 0; } @@ -4772,11 +4800,89 @@ static const struct omap_dss_device_ops dsi_ops = { }, }; +static irqreturn_t omap_dsi_te_irq_handler(int irq, void *dev_id) +{ + struct dsi_data *dsi = (struct dsi_data *)dev_id; + int old; + + old = atomic_cmpxchg(&dsi->do_ext_te_update, 1, 0); + if (old) { + cancel_delayed_work(&dsi->te_timeout_work); + _dsi_update(dsi); + } + + return IRQ_HANDLED; +} + +static void omap_dsi_te_timeout_work_callback(struct work_struct *work) +{ + struct dsi_data *dsi = container_of(work, struct dsi_data, + te_timeout_work.work); + int old; + + old = atomic_cmpxchg(&dsi->do_ext_te_update, 1, 0); + if (old) { + dev_err(dsi->dev, "TE not received for 250ms!\n"); + _dsi_update(dsi); + } +} + +static int omap_dsi_register_te_irq(struct dsi_data *dsi, + struct mipi_dsi_device *client) +{ + int err; + int te_irq; + + dsi->te_gpio = gpiod_get_from_of_node(client->dev.of_node, + "te-gpios", 0, GPIOD_IN, + "dsi-tearing-effect"); + if (IS_ERR(dsi->te_gpio)) { + err = PTR_ERR(dsi->te_gpio); + + if (err == -ENOENT) { + dsi->te_gpio = NULL; + return 0; + } + + dev_err(dsi->dev, "Could not get TE gpio: %d\n", err); + return err; + } + + te_irq = gpiod_to_irq(dsi->te_gpio); + irq_set_status_flags(te_irq, IRQ_NOAUTOEN); + + err = request_threaded_irq(te_irq, omap_dsi_te_irq_handler, NULL, + IRQF_TRIGGER_RISING, "TE", dsi); + if (err) { + dev_err(dsi->dev, "request irq failed with %d\n", err); + gpiod_put(dsi->te_gpio); + return err; + } + + INIT_DEFERRABLE_WORK(&dsi->te_timeout_work, + omap_dsi_te_timeout_work_callback); + + dev_dbg(dsi->dev, "Using GPIO TE\n"); + + return 0; +} + +static void omap_dsi_unregister_te_irq(struct dsi_data *dsi) +{ + if (dsi->te_gpio) { + free_irq(gpiod_to_irq(dsi->te_gpio), dsi); + cancel_delayed_work(&dsi->te_timeout_work); + gpiod_put(dsi->te_gpio); + dsi->te_gpio = NULL; + } +} + static int omap_dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *client) { struct dsi_data *dsi = host_to_omap(host); unsigned int channel = client->channel; + int r; if (channel > 3) return -EINVAL; @@ -4791,13 +4897,20 @@ static int omap_dsi_host_attach(struct mipi_dsi_host *host, return -EINVAL; } - dsi->vc[channel].dest = client; + atomic_set(&dsi->do_ext_te_update, 0); - dsi->pix_fmt = client->format; - if (client->mode_flags & MIPI_DSI_MODE_VIDEO) + if (client->mode_flags & MIPI_DSI_MODE_VIDEO) { dsi->mode = OMAP_DSS_DSI_VIDEO_MODE; - else + } else { + r = omap_dsi_register_te_irq(dsi, client); + if (r) + return r; + dsi->mode = OMAP_DSS_DSI_CMD_MODE; + } + + dsi->vc[channel].dest = client; + dsi->pix_fmt = client->format; return 0; } @@ -4814,6 +4927,7 @@ static int omap_dsi_host_detach(struct mipi_dsi_host *host, if (dsi->vc[channel].dest != client) return -EINVAL; + omap_dsi_unregister_te_irq(dsi); dsi->vc[channel].dest = NULL; return 0; }