| Message ID | 20201124124538.660710-1-tomi.valkeinen@ti.com |
|---|---|
| Headers | show |
| Series | Convert DSI code to use drm_mipi_dsi and drm_panel | expand |
Hi Sam, On 24/11/2020 18:18, Sam Ravnborg wrote: > Hi Tomi, > > On Tue, Nov 24, 2020 at 02:45:13PM +0200, Tomi Valkeinen wrote: >> Use the common MIPI_DCS_GET_ERROR_COUNT_ON_DSI define instead of >> driver's own. >> > They are both 5 - OK > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > > IMO you should get all the patches at least up including this patch applied. > They are all reviewed/acked. And then you have a much smaller stack of > patches to spam us with. Yes, I think that makes sense. I did not want to merge them earlier, as with the v3, I could not get videomode panels work at all (while cmd mode panels did work). So I was not sure if something is totally silly and broken in the series. Now that I can get video mode panels work with some hacks on top, I'm fine with merging these. But it's too late for 5.11, as we need testing and work on the video mode panels. So targeting 5.12. Tomi
On Tue, Nov 24, 2020 at 02:45:28PM +0200, Tomi Valkeinen wrote: > Drop unneeded includes. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> I trust the compiler here. Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > --- > drivers/gpu/drm/panel/panel-dsi-cm.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c > index ec87b785871f..91ed8237a1c2 100644 > --- a/drivers/gpu/drm/panel/panel-dsi-cm.c > +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c > @@ -9,12 +9,7 @@ > #include <linux/backlight.h> > #include <linux/delay.h> > #include <linux/gpio/consumer.h> > -#include <linux/interrupt.h> > #include <linux/jiffies.h> > -#include <linux/module.h> > -#include <linux/sched/signal.h> > -#include <linux/slab.h> > -#include <linux/of_device.h> > #include <linux/regulator/consumer.h> > > #include <drm/drm_connector.h> > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 24/11/2020 18:38, Sam Ravnborg wrote: >>> IMO you should get all the patches at least up including this patch applied. >>> They are all reviewed/acked. And then you have a much smaller stack of >>> patches to spam us with. >> >> Yes, I think that makes sense. I did not want to merge them earlier, as with the v3, I could not get >> videomode panels work at all (while cmd mode panels did work). So I was not sure if something is >> totally silly and broken in the series. >> >> Now that I can get video mode panels work with some hacks on top, I'm fine with merging these. >> >> But it's too late for 5.11, as we need testing and work on the video mode panels. So targeting 5.12. > Obviously your call, but I see no reason to wait for working videomode > panles if what you have now do not introduce any (known) regressions. > > ofc I assume videomode panels is something new and not something that worked > before. It gets a bit muddy here. The omap dsi host driver has had videomode support for a long time, but there has been no upstream videomode panel drivers (omapdrm specific drivers, as omapdrm had its own panel framework) and no board dts files using it. I have a board with a custom made DSI videomode panel setup, but it's broken (cable, I think) and works only randomly. I have an old 4.14 based branch with a hacky panel driver and dts file which get the panel working. I don't know if videomode works on current upstream, or has it been broken between 4.14 and current upstream, as the 4.14 panel driver doesn't work without modifications on current upstream. In this series we convert the omap dsi host driver to be a proper DRM citizen, removing support for omapdrm specific panels, so new DRM panel drivers are needed to replace the omapdrm specific ones. With this series applied, and adding a new panel driver and dts changes, videomode works (Nikolaus confirmed that his panel works. Mine doesn't, as afaics it needs more finetuned initialization which may not be possible with the current DRM bridge/panel callbacks. But mine works with some hacks). But I'm sure in the middle of this series videomode won't work. So, I think one can argue that this causes regressions in the middle of the series to non-upstream panel drivers, but at the end of the series, they probably work, presuming you have a new DRM panel driver for it. Tomi
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:13PM +0200, Tomi Valkeinen wrote: > Use the common MIPI_DCS_GET_ERROR_COUNT_ON_DSI define instead of > driver's own. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/panel/panel-dsi-cm.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c > index 35a0c7da1974..cb0d27a38555 100644 > --- a/drivers/gpu/drm/panel/panel-dsi-cm.c > +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c > @@ -27,7 +27,6 @@ > #include <video/of_display_timing.h> > #include <video/videomode.h> > > -#define DCS_READ_NUM_ERRORS 0x05 > #define DCS_GET_ID1 0xda > #define DCS_GET_ID2 0xdb > #define DCS_GET_ID3 0xdc > @@ -225,7 +224,7 @@ static ssize_t num_dsi_errors_show(struct device *dev, > mutex_lock(&ddata->lock); > > if (ddata->enabled) > - r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS, &errors); > + r = dsicm_dcs_read_1(ddata, MIPI_DCS_GET_ERROR_COUNT_ON_DSI, &errors); > > mutex_unlock(&ddata->lock); >
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:15PM +0200, Tomi Valkeinen wrote: > Add address-cells & size-cells to DSI nodes so that board files do not > need to define them. How about adding ports too, while at it ? It would also be nice to convert the DT bindings to YAML :-) > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Tony Lindgren <tony@atomide.com> > --- > arch/arm/boot/dts/omap5.dtsi | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi > index 2bf2e5839a7f..e6f6947965ef 100644 > --- a/arch/arm/boot/dts/omap5.dtsi > +++ b/arch/arm/boot/dts/omap5.dtsi > @@ -517,6 +517,9 @@ dsi1: encoder@0 { > clocks = <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 8>, > <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>; > clock-names = "fck", "sys_clk"; > + > + #address-cells = <1>; > + #size-cells = <0>; > }; > }; > > @@ -549,6 +552,9 @@ dsi2: encoder@0 { > clocks = <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 8>, > <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>; > clock-names = "fck", "sys_clk"; > + > + #address-cells = <1>; > + #size-cells = <0>; > }; > }; >
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:17PM +0200, Tomi Valkeinen wrote: > The DSI host driver currently ignores the video mode flags in > client->mode_flags. Add the code to take the transfer mode from client's > mode_flags. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index c3592c6db977..7fee9cf8782d 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -5140,6 +5140,13 @@ static int omap_dsi_host_attach(struct mipi_dsi_host *host, > dsi->config.lp_clk_min = 7000000; // TODO: get from client? > dsi->config.lp_clk_max = client->lp_rate; > > + if (client->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) > + dsi->config.trans_mode = OMAP_DSS_DSI_BURST_MODE; > + else if (client->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) > + dsi->config.trans_mode = OMAP_DSS_DSI_PULSE_MODE; > + else > + dsi->config.trans_mode = OMAP_DSS_DSI_EVENT_MODE; > + > dsi->ulps_auto_idle = false; > > return 0; >
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:19PM +0200, Tomi Valkeinen wrote: > The OMAP DSI command mode panel driver used to send page & column > address before each frame update, and this code was moved into the DSI > host driver when converting it to the DRM bridge model. > > However, it's not really required to send the page & column address > before each frame. It's also something that doesn't really belong to the > DSI host driver, so we should drop the code. > > That said, frame updates break if we don't send _something_ between the > frames. A NOP command does the trick. > > It is not clear if this behavior is as expected from a DSI command mode > frame transfer, or is it a feature/issue with OMAP DSI driver, or a > feature/issue in the command mode panel used. So this probably needs to > be revisited later. This is important information, could you capture it in a comment in the code ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 41 +++++++++---------------------- > 1 file changed, 12 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 7fee9cf8782d..746c2149fbbd 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -3884,35 +3884,19 @@ static int _dsi_update(struct dsi_data *dsi) > return 0; > } > > -static int _dsi_update_window(struct dsi_data *dsi, int channel, > - int x, int y, int w, int h) > -{ > - int x1 = x, x2 = (x + w - 1); > - int y1 = y, y2 = (y + h - 1); > - u8 payloadX[5] = { MIPI_DCS_SET_COLUMN_ADDRESS, > - x1 >> 8, x1 & 0xff, x2 >> 8, x2 & 0xff }; > - u8 payloadY[5] = { MIPI_DCS_SET_PAGE_ADDRESS, > - y1 >> 8, y1 & 0xff, y2 >> 8, y2 & 0xff }; > - struct mipi_dsi_msg msgX = { 0 }, msgY = { 0 }; > - int ret; > +static int _dsi_send_nop(struct dsi_data *dsi, int channel) > +{ > + const u8 payload[] = { MIPI_DCS_NOP }; > + const struct mipi_dsi_msg msg = { > + .channel = channel, > + .type = MIPI_DSI_DCS_SHORT_WRITE, > + .tx_len = 1, > + .tx_buf = payload, > + }; > > WARN_ON(!dsi_bus_is_locked(dsi)); > > - msgX.type = MIPI_DSI_DCS_LONG_WRITE; > - msgX.channel = channel; > - msgX.tx_buf = payloadX; > - msgX.tx_len = sizeof(payloadX); > - > - msgY.type = MIPI_DSI_DCS_LONG_WRITE; > - msgY.channel = channel; > - msgY.tx_buf = payloadY; > - msgY.tx_len = sizeof(payloadY); > - > - ret = _omap_dsi_host_transfer(dsi, &msgX); > - if (ret != 0) > - return ret; > - > - return _omap_dsi_host_transfer(dsi, &msgY); > + return _omap_dsi_host_transfer(dsi, &msg); > } > > static int dsi_update_channel(struct omap_dss_device *dssdev, int channel) > @@ -3944,10 +3928,9 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int channel) > > dsi_set_ulps_auto(dsi, false); > > - r = _dsi_update_window(dsi, channel, 0, 0, dsi->vm.hactive, > - dsi->vm.vactive); > + r = _dsi_send_nop(dsi, channel); > if (r < 0) { > - DSSWARN("window update error: %d\n", r); > + DSSWARN("failed to send nop between frames: %d\n", r); > goto err; > } >
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:21PM +0200, Tomi Valkeinen wrote: > A DSI peripheral can have virtual channel ID of 0-3. This should be > always the case, and there's no need in the driver to validate the > channel. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 63338324c564..fbf05097612f 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -3902,9 +3902,6 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int channel) > struct dsi_data *dsi = to_dsi_data(dssdev); > int r; > > - if (channel > 3) > - return -EINVAL; > - > dsi_bus_lock(dsi); > > if (!dsi->video_enabled) { > @@ -5063,12 +5060,8 @@ 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; > - > if (dsi->dsidev) { > DSSERR("dsi client already attached\n"); > return -EBUSY; > @@ -5118,10 +5111,6 @@ static int omap_dsi_host_detach(struct mipi_dsi_host *host, > struct mipi_dsi_device *client) > { > struct dsi_data *dsi = host_to_omap(host); > - unsigned int channel = client->channel; > - > - if (channel > 3) > - return -EINVAL; > > if (WARN_ON(dsi->dsidev != client)) > return -EINVAL;
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:25PM +0200, Tomi Valkeinen wrote: > For command mode panels we can use a single VC for sending command and > video data, even if we have to change the data source for that VC when > going from command to video or vice versa. > > However, with video mode panels we want to keep the pixel data VC > enabled, and use another VC for command data, and the commands will get > interleaved into the pixel data. > > This patch makes the driver use VC0 for commands and VC1 for video. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 41d6231d6e31..019814a0a264 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -452,7 +452,9 @@ static bool dsi_perf; > module_param(dsi_perf, bool, 0644); > #endif > > -#define VC_DEFAULT 0 > +/* Note: for some reason video mode seems to work only if VC_VIDEO is 0 */ > +#define VC_VIDEO 0 > +#define VC_CMD 1 > > #define drm_bridge_to_dsi(bridge) \ > container_of(bridge, struct dsi_data, bridge) > @@ -3723,7 +3725,7 @@ static void dsi_disable_video_outputs(struct omap_dss_device *dssdev) > dsi_bus_lock(dsi); > dsi->video_enabled = false; > > - dsi_disable_video_output(dssdev, VC_DEFAULT); > + dsi_disable_video_output(dssdev, VC_VIDEO); > > dsi_display_disable(dssdev); > > @@ -3946,7 +3948,7 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int vc) > > static int dsi_update_all(struct omap_dss_device *dssdev) > { > - return dsi_update_channel(dssdev, VC_DEFAULT); > + return dsi_update_channel(dssdev, VC_VIDEO); > } > > /* Display funcs */ > @@ -4179,7 +4181,7 @@ static void dsi_enable_video_outputs(struct omap_dss_device *dssdev) > > dsi_display_enable(dssdev); > > - dsi_enable_video_output(dssdev, VC_DEFAULT); > + dsi_enable_video_output(dssdev, VC_VIDEO); > > dsi->video_enabled = true; > > @@ -4936,7 +4938,7 @@ static ssize_t omap_dsi_host_transfer(struct mipi_dsi_host *host, > { > struct dsi_data *dsi = host_to_omap(host); > int r; > - int vc = VC_DEFAULT; > + int vc = VC_CMD; > > dsi_bus_lock(dsi); >
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:29PM +0200, Tomi Valkeinen wrote: > Move structs and defines to a private dsi.h header file to make dsi.c a > bit easier to navigate. Also move the (now) private structs and defines > from omapdss.h to dsi.h. I usually tend to keep structures used by a single .c file in that file, but it's a matter of personal preference I suppose. > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 384 +--------------------- > drivers/gpu/drm/omapdrm/dss/dsi.h | 456 ++++++++++++++++++++++++++ > drivers/gpu/drm/omapdrm/dss/omapdss.h | 64 ---- > 3 files changed, 457 insertions(+), 447 deletions(-) > create mode 100644 drivers/gpu/drm/omapdrm/dss/dsi.h > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 019814a0a264..a01e09c9b477 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -45,73 +45,7 @@ > > #define DSI_CATCH_MISSING_TE > > -struct dsi_reg { u16 module; u16 idx; }; > - > -#define DSI_REG(mod, idx) ((const struct dsi_reg) { mod, idx }) > - > -/* DSI Protocol Engine */ > - > -#define DSI_PROTO 0 > -#define DSI_PROTO_SZ 0x200 > - > -#define DSI_REVISION DSI_REG(DSI_PROTO, 0x0000) > -#define DSI_SYSCONFIG DSI_REG(DSI_PROTO, 0x0010) > -#define DSI_SYSSTATUS DSI_REG(DSI_PROTO, 0x0014) > -#define DSI_IRQSTATUS DSI_REG(DSI_PROTO, 0x0018) > -#define DSI_IRQENABLE DSI_REG(DSI_PROTO, 0x001C) > -#define DSI_CTRL DSI_REG(DSI_PROTO, 0x0040) > -#define DSI_GNQ DSI_REG(DSI_PROTO, 0x0044) > -#define DSI_COMPLEXIO_CFG1 DSI_REG(DSI_PROTO, 0x0048) > -#define DSI_COMPLEXIO_IRQ_STATUS DSI_REG(DSI_PROTO, 0x004C) > -#define DSI_COMPLEXIO_IRQ_ENABLE DSI_REG(DSI_PROTO, 0x0050) > -#define DSI_CLK_CTRL DSI_REG(DSI_PROTO, 0x0054) > -#define DSI_TIMING1 DSI_REG(DSI_PROTO, 0x0058) > -#define DSI_TIMING2 DSI_REG(DSI_PROTO, 0x005C) > -#define DSI_VM_TIMING1 DSI_REG(DSI_PROTO, 0x0060) > -#define DSI_VM_TIMING2 DSI_REG(DSI_PROTO, 0x0064) > -#define DSI_VM_TIMING3 DSI_REG(DSI_PROTO, 0x0068) > -#define DSI_CLK_TIMING DSI_REG(DSI_PROTO, 0x006C) > -#define DSI_TX_FIFO_VC_SIZE DSI_REG(DSI_PROTO, 0x0070) > -#define DSI_RX_FIFO_VC_SIZE DSI_REG(DSI_PROTO, 0x0074) > -#define DSI_COMPLEXIO_CFG2 DSI_REG(DSI_PROTO, 0x0078) > -#define DSI_RX_FIFO_VC_FULLNESS DSI_REG(DSI_PROTO, 0x007C) > -#define DSI_VM_TIMING4 DSI_REG(DSI_PROTO, 0x0080) > -#define DSI_TX_FIFO_VC_EMPTINESS DSI_REG(DSI_PROTO, 0x0084) > -#define DSI_VM_TIMING5 DSI_REG(DSI_PROTO, 0x0088) > -#define DSI_VM_TIMING6 DSI_REG(DSI_PROTO, 0x008C) > -#define DSI_VM_TIMING7 DSI_REG(DSI_PROTO, 0x0090) > -#define DSI_STOPCLK_TIMING DSI_REG(DSI_PROTO, 0x0094) > -#define DSI_VC_CTRL(n) DSI_REG(DSI_PROTO, 0x0100 + (n * 0x20)) > -#define DSI_VC_TE(n) DSI_REG(DSI_PROTO, 0x0104 + (n * 0x20)) > -#define DSI_VC_LONG_PACKET_HEADER(n) DSI_REG(DSI_PROTO, 0x0108 + (n * 0x20)) > -#define DSI_VC_LONG_PACKET_PAYLOAD(n) DSI_REG(DSI_PROTO, 0x010C + (n * 0x20)) > -#define DSI_VC_SHORT_PACKET_HEADER(n) DSI_REG(DSI_PROTO, 0x0110 + (n * 0x20)) > -#define DSI_VC_IRQSTATUS(n) DSI_REG(DSI_PROTO, 0x0118 + (n * 0x20)) > -#define DSI_VC_IRQENABLE(n) DSI_REG(DSI_PROTO, 0x011C + (n * 0x20)) > - > -/* DSIPHY_SCP */ > - > -#define DSI_PHY 1 > -#define DSI_PHY_OFFSET 0x200 > -#define DSI_PHY_SZ 0x40 > - > -#define DSI_DSIPHY_CFG0 DSI_REG(DSI_PHY, 0x0000) > -#define DSI_DSIPHY_CFG1 DSI_REG(DSI_PHY, 0x0004) > -#define DSI_DSIPHY_CFG2 DSI_REG(DSI_PHY, 0x0008) > -#define DSI_DSIPHY_CFG5 DSI_REG(DSI_PHY, 0x0014) > -#define DSI_DSIPHY_CFG10 DSI_REG(DSI_PHY, 0x0028) > - > -/* DSI_PLL_CTRL_SCP */ > - > -#define DSI_PLL 2 > -#define DSI_PLL_OFFSET 0x300 > -#define DSI_PLL_SZ 0x20 > - > -#define DSI_PLL_CONTROL DSI_REG(DSI_PLL, 0x0000) > -#define DSI_PLL_STATUS DSI_REG(DSI_PLL, 0x0004) > -#define DSI_PLL_GO DSI_REG(DSI_PLL, 0x0008) > -#define DSI_PLL_CONFIGURATION1 DSI_REG(DSI_PLL, 0x000C) > -#define DSI_PLL_CONFIGURATION2 DSI_REG(DSI_PLL, 0x0010) > +#include "dsi.h" > > #define REG_GET(dsi, idx, start, end) \ > FLD_GET(dsi_read_reg(dsi, idx), start, end) > @@ -119,96 +53,6 @@ struct dsi_reg { u16 module; u16 idx; }; > #define REG_FLD_MOD(dsi, idx, val, start, end) \ > dsi_write_reg(dsi, idx, FLD_MOD(dsi_read_reg(dsi, idx), val, start, end)) > > -/* Global interrupts */ > -#define DSI_IRQ_VC0 (1 << 0) > -#define DSI_IRQ_VC1 (1 << 1) > -#define DSI_IRQ_VC2 (1 << 2) > -#define DSI_IRQ_VC3 (1 << 3) > -#define DSI_IRQ_WAKEUP (1 << 4) > -#define DSI_IRQ_RESYNC (1 << 5) > -#define DSI_IRQ_PLL_LOCK (1 << 7) > -#define DSI_IRQ_PLL_UNLOCK (1 << 8) > -#define DSI_IRQ_PLL_RECALL (1 << 9) > -#define DSI_IRQ_COMPLEXIO_ERR (1 << 10) > -#define DSI_IRQ_HS_TX_TIMEOUT (1 << 14) > -#define DSI_IRQ_LP_RX_TIMEOUT (1 << 15) > -#define DSI_IRQ_TE_TRIGGER (1 << 16) > -#define DSI_IRQ_ACK_TRIGGER (1 << 17) > -#define DSI_IRQ_SYNC_LOST (1 << 18) > -#define DSI_IRQ_LDO_POWER_GOOD (1 << 19) > -#define DSI_IRQ_TA_TIMEOUT (1 << 20) > -#define DSI_IRQ_ERROR_MASK \ > - (DSI_IRQ_HS_TX_TIMEOUT | DSI_IRQ_LP_RX_TIMEOUT | DSI_IRQ_SYNC_LOST | \ > - DSI_IRQ_TA_TIMEOUT) > -#define DSI_IRQ_CHANNEL_MASK 0xf > - > -/* Virtual channel interrupts */ > -#define DSI_VC_IRQ_CS (1 << 0) > -#define DSI_VC_IRQ_ECC_CORR (1 << 1) > -#define DSI_VC_IRQ_PACKET_SENT (1 << 2) > -#define DSI_VC_IRQ_FIFO_TX_OVF (1 << 3) > -#define DSI_VC_IRQ_FIFO_RX_OVF (1 << 4) > -#define DSI_VC_IRQ_BTA (1 << 5) > -#define DSI_VC_IRQ_ECC_NO_CORR (1 << 6) > -#define DSI_VC_IRQ_FIFO_TX_UDF (1 << 7) > -#define DSI_VC_IRQ_PP_BUSY_CHANGE (1 << 8) > -#define DSI_VC_IRQ_ERROR_MASK \ > - (DSI_VC_IRQ_CS | DSI_VC_IRQ_ECC_CORR | DSI_VC_IRQ_FIFO_TX_OVF | \ > - DSI_VC_IRQ_FIFO_RX_OVF | DSI_VC_IRQ_ECC_NO_CORR | \ > - DSI_VC_IRQ_FIFO_TX_UDF) > - > -/* ComplexIO interrupts */ > -#define DSI_CIO_IRQ_ERRSYNCESC1 (1 << 0) > -#define DSI_CIO_IRQ_ERRSYNCESC2 (1 << 1) > -#define DSI_CIO_IRQ_ERRSYNCESC3 (1 << 2) > -#define DSI_CIO_IRQ_ERRSYNCESC4 (1 << 3) > -#define DSI_CIO_IRQ_ERRSYNCESC5 (1 << 4) > -#define DSI_CIO_IRQ_ERRESC1 (1 << 5) > -#define DSI_CIO_IRQ_ERRESC2 (1 << 6) > -#define DSI_CIO_IRQ_ERRESC3 (1 << 7) > -#define DSI_CIO_IRQ_ERRESC4 (1 << 8) > -#define DSI_CIO_IRQ_ERRESC5 (1 << 9) > -#define DSI_CIO_IRQ_ERRCONTROL1 (1 << 10) > -#define DSI_CIO_IRQ_ERRCONTROL2 (1 << 11) > -#define DSI_CIO_IRQ_ERRCONTROL3 (1 << 12) > -#define DSI_CIO_IRQ_ERRCONTROL4 (1 << 13) > -#define DSI_CIO_IRQ_ERRCONTROL5 (1 << 14) > -#define DSI_CIO_IRQ_STATEULPS1 (1 << 15) > -#define DSI_CIO_IRQ_STATEULPS2 (1 << 16) > -#define DSI_CIO_IRQ_STATEULPS3 (1 << 17) > -#define DSI_CIO_IRQ_STATEULPS4 (1 << 18) > -#define DSI_CIO_IRQ_STATEULPS5 (1 << 19) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP0_1 (1 << 20) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP1_1 (1 << 21) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP0_2 (1 << 22) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP1_2 (1 << 23) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP0_3 (1 << 24) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP1_3 (1 << 25) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP0_4 (1 << 26) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP1_4 (1 << 27) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP0_5 (1 << 28) > -#define DSI_CIO_IRQ_ERRCONTENTIONLP1_5 (1 << 29) > -#define DSI_CIO_IRQ_ULPSACTIVENOT_ALL0 (1 << 30) > -#define DSI_CIO_IRQ_ULPSACTIVENOT_ALL1 (1 << 31) > -#define DSI_CIO_IRQ_ERROR_MASK \ > - (DSI_CIO_IRQ_ERRSYNCESC1 | DSI_CIO_IRQ_ERRSYNCESC2 | \ > - DSI_CIO_IRQ_ERRSYNCESC3 | DSI_CIO_IRQ_ERRSYNCESC4 | \ > - DSI_CIO_IRQ_ERRSYNCESC5 | \ > - DSI_CIO_IRQ_ERRESC1 | DSI_CIO_IRQ_ERRESC2 | \ > - DSI_CIO_IRQ_ERRESC3 | DSI_CIO_IRQ_ERRESC4 | \ > - DSI_CIO_IRQ_ERRESC5 | \ > - DSI_CIO_IRQ_ERRCONTROL1 | DSI_CIO_IRQ_ERRCONTROL2 | \ > - DSI_CIO_IRQ_ERRCONTROL3 | DSI_CIO_IRQ_ERRCONTROL4 | \ > - DSI_CIO_IRQ_ERRCONTROL5 | \ > - DSI_CIO_IRQ_ERRCONTENTIONLP0_1 | DSI_CIO_IRQ_ERRCONTENTIONLP1_1 | \ > - DSI_CIO_IRQ_ERRCONTENTIONLP0_2 | DSI_CIO_IRQ_ERRCONTENTIONLP1_2 | \ > - DSI_CIO_IRQ_ERRCONTENTIONLP0_3 | DSI_CIO_IRQ_ERRCONTENTIONLP1_3 | \ > - DSI_CIO_IRQ_ERRCONTENTIONLP0_4 | DSI_CIO_IRQ_ERRCONTENTIONLP1_4 | \ > - DSI_CIO_IRQ_ERRCONTENTIONLP0_5 | DSI_CIO_IRQ_ERRCONTENTIONLP1_5) > - > -typedef void (*omap_dsi_isr_t) (void *arg, u32 mask); > -struct dsi_data; > - > static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable); > > static int dsi_display_init_dispc(struct dsi_data *dsi); > @@ -221,232 +65,6 @@ static ssize_t _omap_dsi_host_transfer(struct dsi_data *dsi, int vc, > > static void dsi_display_disable(struct omap_dss_device *dssdev); > > -/* DSI PLL HSDIV indices */ > -#define HSDIV_DISPC 0 > -#define HSDIV_DSI 1 > - > -#define DSI_MAX_NR_ISRS 2 > -#define DSI_MAX_NR_LANES 5 > - > -enum dsi_model { > - DSI_MODEL_OMAP3, > - DSI_MODEL_OMAP4, > - DSI_MODEL_OMAP5, > -}; > - > -enum dsi_lane_function { > - DSI_LANE_UNUSED = 0, > - DSI_LANE_CLK, > - DSI_LANE_DATA1, > - DSI_LANE_DATA2, > - DSI_LANE_DATA3, > - DSI_LANE_DATA4, > -}; > - > -struct dsi_lane_config { > - enum dsi_lane_function function; > - u8 polarity; > -}; > - > -struct dsi_isr_data { > - omap_dsi_isr_t isr; > - void *arg; > - u32 mask; > -}; > - > -enum fifo_size { > - DSI_FIFO_SIZE_0 = 0, > - DSI_FIFO_SIZE_32 = 1, > - DSI_FIFO_SIZE_64 = 2, > - DSI_FIFO_SIZE_96 = 3, > - DSI_FIFO_SIZE_128 = 4, > -}; > - > -enum dsi_vc_source { > - DSI_VC_SOURCE_L4 = 0, > - DSI_VC_SOURCE_VP, > -}; > - > -struct dsi_irq_stats { > - unsigned long last_reset; > - unsigned int irq_count; > - unsigned int dsi_irqs[32]; > - unsigned int vc_irqs[4][32]; > - unsigned int cio_irqs[32]; > -}; > - > -struct dsi_isr_tables { > - struct dsi_isr_data isr_table[DSI_MAX_NR_ISRS]; > - struct dsi_isr_data isr_table_vc[4][DSI_MAX_NR_ISRS]; > - struct dsi_isr_data isr_table_cio[DSI_MAX_NR_ISRS]; > -}; > - > -struct dsi_lp_clock_info { > - unsigned long lp_clk; > - u16 lp_clk_div; > -}; > - > -struct dsi_clk_calc_ctx { > - struct dsi_data *dsi; > - struct dss_pll *pll; > - > - /* inputs */ > - > - const struct omap_dss_dsi_config *config; > - > - unsigned long req_pck_min, req_pck_nom, req_pck_max; > - > - /* outputs */ > - > - struct dss_pll_clock_info dsi_cinfo; > - struct dispc_clock_info dispc_cinfo; > - struct dsi_lp_clock_info lp_cinfo; > - > - struct videomode vm; > - struct omap_dss_dsi_videomode_timings dsi_vm; > -}; > - > -struct dsi_module_id_data { > - u32 address; > - int id; > -}; > - > -enum dsi_quirks { > - DSI_QUIRK_PLL_PWR_BUG = (1 << 0), /* DSI-PLL power command 0x3 is not working */ > - DSI_QUIRK_DCS_CMD_CONFIG_VC = (1 << 1), > - DSI_QUIRK_VC_OCP_WIDTH = (1 << 2), > - DSI_QUIRK_REVERSE_TXCLKESC = (1 << 3), > - DSI_QUIRK_GNQ = (1 << 4), > - DSI_QUIRK_PHY_DCC = (1 << 5), > -}; > - > -struct dsi_of_data { > - enum dsi_model model; > - const struct dss_pll_hw *pll_hw; > - const struct dsi_module_id_data *modules; > - unsigned int max_fck_freq; > - unsigned int max_pll_lpdiv; > - enum dsi_quirks quirks; > -}; > - > -struct dsi_data { > - struct device *dev; > - void __iomem *proto_base; > - void __iomem *phy_base; > - void __iomem *pll_base; > - > - const struct dsi_of_data *data; > - int module_id; > - > - int irq; > - > - bool is_enabled; > - > - struct clk *dss_clk; > - struct regmap *syscon; > - struct dss_device *dss; > - > - struct mipi_dsi_host host; > - > - struct dispc_clock_info user_dispc_cinfo; > - struct dss_pll_clock_info user_dsi_cinfo; > - > - struct dsi_lp_clock_info user_lp_cinfo; > - struct dsi_lp_clock_info current_lp_cinfo; > - > - struct dss_pll pll; > - > - bool vdds_dsi_enabled; > - struct regulator *vdds_dsi_reg; > - > - struct mipi_dsi_device *dsidev; > - > - struct { > - enum dsi_vc_source source; > - enum fifo_size tx_fifo_size; > - enum fifo_size rx_fifo_size; > - } vc[4]; > - > - struct mutex lock; > - struct semaphore bus_lock; > - > - spinlock_t irq_lock; > - struct dsi_isr_tables isr_tables; > - /* space for a copy used by the interrupt handler */ > - struct dsi_isr_tables isr_tables_copy; > - > - int update_vc; > -#ifdef DSI_PERF_MEASURE > - unsigned int update_bytes; > -#endif > - > - /* external TE GPIO */ > - struct gpio_desc *te_gpio; > - int te_irq; > - struct delayed_work te_timeout_work; > - atomic_t do_ext_te_update; > - > - bool te_enabled; > - bool ulps_enabled; > - bool ulps_auto_idle; > - bool video_enabled; > - > - struct delayed_work ulps_work; > - > - struct delayed_work framedone_timeout_work; > - > -#ifdef DSI_CATCH_MISSING_TE > - struct timer_list te_timer; > -#endif > - > - unsigned long cache_req_pck; > - unsigned long cache_clk_freq; > - struct dss_pll_clock_info cache_cinfo; > - > - u32 errors; > - spinlock_t errors_lock; > -#ifdef DSI_PERF_MEASURE > - ktime_t perf_setup_time; > - ktime_t perf_start_time; > -#endif > - int debug_read; > - int debug_write; > - struct { > - struct dss_debugfs_entry *irqs; > - struct dss_debugfs_entry *regs; > - struct dss_debugfs_entry *clks; > - } debugfs; > - > -#ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS > - spinlock_t irq_stats_lock; > - struct dsi_irq_stats irq_stats; > -#endif > - > - unsigned int num_lanes_supported; > - unsigned int line_buffer_size; > - > - struct dsi_lane_config lanes[DSI_MAX_NR_LANES]; > - unsigned int num_lanes_used; > - > - unsigned int scp_clk_refcount; > - > - struct omap_dss_dsi_config config; > - > - struct dss_lcd_mgr_config mgr_config; > - struct videomode vm; > - enum mipi_dsi_pixel_format pix_fmt; > - enum omap_dss_dsi_mode mode; > - struct omap_dss_dsi_videomode_timings vm_timings; > - > - struct omap_dss_device output; > - struct drm_bridge bridge; > -}; > - > -struct dsi_packet_sent_handler_data { > - struct dsi_data *dsi; > - struct completion *completion; > -}; > - > #ifdef DSI_PERF_MEASURE > static bool dsi_perf; > module_param(dsi_perf, bool, 0644); > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.h b/drivers/gpu/drm/omapdrm/dss/dsi.h > new file mode 100644 > index 000000000000..7cc2cc748ed9 > --- /dev/null > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.h > @@ -0,0 +1,456 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (C) 2016 Texas Instruments Incorporated - http://www.ti.com/ > + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > + */ > + > +#ifndef __OMAP_DRM_DSS_DSI_H > +#define __OMAP_DRM_DSS_DSI_H > + > +#include <drm/drm_mipi_dsi.h> > + > +struct dsi_reg { u16 module; u16 idx; }; How about using the common kernel coding style ? struct dsi_reg { u16 module; u16 idx; }; Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > +#define DSI_REG(mod, idx) ((const struct dsi_reg) { mod, idx }) > + > +/* DSI Protocol Engine */ > + > +#define DSI_PROTO 0 > +#define DSI_PROTO_SZ 0x200 > + > +#define DSI_REVISION DSI_REG(DSI_PROTO, 0x0000) > +#define DSI_SYSCONFIG DSI_REG(DSI_PROTO, 0x0010) > +#define DSI_SYSSTATUS DSI_REG(DSI_PROTO, 0x0014) > +#define DSI_IRQSTATUS DSI_REG(DSI_PROTO, 0x0018) > +#define DSI_IRQENABLE DSI_REG(DSI_PROTO, 0x001C) > +#define DSI_CTRL DSI_REG(DSI_PROTO, 0x0040) > +#define DSI_GNQ DSI_REG(DSI_PROTO, 0x0044) > +#define DSI_COMPLEXIO_CFG1 DSI_REG(DSI_PROTO, 0x0048) > +#define DSI_COMPLEXIO_IRQ_STATUS DSI_REG(DSI_PROTO, 0x004C) > +#define DSI_COMPLEXIO_IRQ_ENABLE DSI_REG(DSI_PROTO, 0x0050) > +#define DSI_CLK_CTRL DSI_REG(DSI_PROTO, 0x0054) > +#define DSI_TIMING1 DSI_REG(DSI_PROTO, 0x0058) > +#define DSI_TIMING2 DSI_REG(DSI_PROTO, 0x005C) > +#define DSI_VM_TIMING1 DSI_REG(DSI_PROTO, 0x0060) > +#define DSI_VM_TIMING2 DSI_REG(DSI_PROTO, 0x0064) > +#define DSI_VM_TIMING3 DSI_REG(DSI_PROTO, 0x0068) > +#define DSI_CLK_TIMING DSI_REG(DSI_PROTO, 0x006C) > +#define DSI_TX_FIFO_VC_SIZE DSI_REG(DSI_PROTO, 0x0070) > +#define DSI_RX_FIFO_VC_SIZE DSI_REG(DSI_PROTO, 0x0074) > +#define DSI_COMPLEXIO_CFG2 DSI_REG(DSI_PROTO, 0x0078) > +#define DSI_RX_FIFO_VC_FULLNESS DSI_REG(DSI_PROTO, 0x007C) > +#define DSI_VM_TIMING4 DSI_REG(DSI_PROTO, 0x0080) > +#define DSI_TX_FIFO_VC_EMPTINESS DSI_REG(DSI_PROTO, 0x0084) > +#define DSI_VM_TIMING5 DSI_REG(DSI_PROTO, 0x0088) > +#define DSI_VM_TIMING6 DSI_REG(DSI_PROTO, 0x008C) > +#define DSI_VM_TIMING7 DSI_REG(DSI_PROTO, 0x0090) > +#define DSI_STOPCLK_TIMING DSI_REG(DSI_PROTO, 0x0094) > +#define DSI_VC_CTRL(n) DSI_REG(DSI_PROTO, 0x0100 + (n * 0x20)) > +#define DSI_VC_TE(n) DSI_REG(DSI_PROTO, 0x0104 + (n * 0x20)) > +#define DSI_VC_LONG_PACKET_HEADER(n) DSI_REG(DSI_PROTO, 0x0108 + (n * 0x20)) > +#define DSI_VC_LONG_PACKET_PAYLOAD(n) DSI_REG(DSI_PROTO, 0x010C + (n * 0x20)) > +#define DSI_VC_SHORT_PACKET_HEADER(n) DSI_REG(DSI_PROTO, 0x0110 + (n * 0x20)) > +#define DSI_VC_IRQSTATUS(n) DSI_REG(DSI_PROTO, 0x0118 + (n * 0x20)) > +#define DSI_VC_IRQENABLE(n) DSI_REG(DSI_PROTO, 0x011C + (n * 0x20)) > + > +/* DSIPHY_SCP */ > + > +#define DSI_PHY 1 > +#define DSI_PHY_OFFSET 0x200 > +#define DSI_PHY_SZ 0x40 > + > +#define DSI_DSIPHY_CFG0 DSI_REG(DSI_PHY, 0x0000) > +#define DSI_DSIPHY_CFG1 DSI_REG(DSI_PHY, 0x0004) > +#define DSI_DSIPHY_CFG2 DSI_REG(DSI_PHY, 0x0008) > +#define DSI_DSIPHY_CFG5 DSI_REG(DSI_PHY, 0x0014) > +#define DSI_DSIPHY_CFG10 DSI_REG(DSI_PHY, 0x0028) > + > +/* DSI_PLL_CTRL_SCP */ > + > +#define DSI_PLL 2 > +#define DSI_PLL_OFFSET 0x300 > +#define DSI_PLL_SZ 0x20 > + > +#define DSI_PLL_CONTROL DSI_REG(DSI_PLL, 0x0000) > +#define DSI_PLL_STATUS DSI_REG(DSI_PLL, 0x0004) > +#define DSI_PLL_GO DSI_REG(DSI_PLL, 0x0008) > +#define DSI_PLL_CONFIGURATION1 DSI_REG(DSI_PLL, 0x000C) > +#define DSI_PLL_CONFIGURATION2 DSI_REG(DSI_PLL, 0x0010) > + > +/* Global interrupts */ > +#define DSI_IRQ_VC0 (1 << 0) > +#define DSI_IRQ_VC1 (1 << 1) > +#define DSI_IRQ_VC2 (1 << 2) > +#define DSI_IRQ_VC3 (1 << 3) > +#define DSI_IRQ_WAKEUP (1 << 4) > +#define DSI_IRQ_RESYNC (1 << 5) > +#define DSI_IRQ_PLL_LOCK (1 << 7) > +#define DSI_IRQ_PLL_UNLOCK (1 << 8) > +#define DSI_IRQ_PLL_RECALL (1 << 9) > +#define DSI_IRQ_COMPLEXIO_ERR (1 << 10) > +#define DSI_IRQ_HS_TX_TIMEOUT (1 << 14) > +#define DSI_IRQ_LP_RX_TIMEOUT (1 << 15) > +#define DSI_IRQ_TE_TRIGGER (1 << 16) > +#define DSI_IRQ_ACK_TRIGGER (1 << 17) > +#define DSI_IRQ_SYNC_LOST (1 << 18) > +#define DSI_IRQ_LDO_POWER_GOOD (1 << 19) > +#define DSI_IRQ_TA_TIMEOUT (1 << 20) > +#define DSI_IRQ_ERROR_MASK \ > + (DSI_IRQ_HS_TX_TIMEOUT | DSI_IRQ_LP_RX_TIMEOUT | DSI_IRQ_SYNC_LOST | \ > + DSI_IRQ_TA_TIMEOUT) > +#define DSI_IRQ_CHANNEL_MASK 0xf > + > +/* Virtual channel interrupts */ > +#define DSI_VC_IRQ_CS (1 << 0) > +#define DSI_VC_IRQ_ECC_CORR (1 << 1) > +#define DSI_VC_IRQ_PACKET_SENT (1 << 2) > +#define DSI_VC_IRQ_FIFO_TX_OVF (1 << 3) > +#define DSI_VC_IRQ_FIFO_RX_OVF (1 << 4) > +#define DSI_VC_IRQ_BTA (1 << 5) > +#define DSI_VC_IRQ_ECC_NO_CORR (1 << 6) > +#define DSI_VC_IRQ_FIFO_TX_UDF (1 << 7) > +#define DSI_VC_IRQ_PP_BUSY_CHANGE (1 << 8) > +#define DSI_VC_IRQ_ERROR_MASK \ > + (DSI_VC_IRQ_CS | DSI_VC_IRQ_ECC_CORR | DSI_VC_IRQ_FIFO_TX_OVF | \ > + DSI_VC_IRQ_FIFO_RX_OVF | DSI_VC_IRQ_ECC_NO_CORR | \ > + DSI_VC_IRQ_FIFO_TX_UDF) > + > +/* ComplexIO interrupts */ > +#define DSI_CIO_IRQ_ERRSYNCESC1 (1 << 0) > +#define DSI_CIO_IRQ_ERRSYNCESC2 (1 << 1) > +#define DSI_CIO_IRQ_ERRSYNCESC3 (1 << 2) > +#define DSI_CIO_IRQ_ERRSYNCESC4 (1 << 3) > +#define DSI_CIO_IRQ_ERRSYNCESC5 (1 << 4) > +#define DSI_CIO_IRQ_ERRESC1 (1 << 5) > +#define DSI_CIO_IRQ_ERRESC2 (1 << 6) > +#define DSI_CIO_IRQ_ERRESC3 (1 << 7) > +#define DSI_CIO_IRQ_ERRESC4 (1 << 8) > +#define DSI_CIO_IRQ_ERRESC5 (1 << 9) > +#define DSI_CIO_IRQ_ERRCONTROL1 (1 << 10) > +#define DSI_CIO_IRQ_ERRCONTROL2 (1 << 11) > +#define DSI_CIO_IRQ_ERRCONTROL3 (1 << 12) > +#define DSI_CIO_IRQ_ERRCONTROL4 (1 << 13) > +#define DSI_CIO_IRQ_ERRCONTROL5 (1 << 14) > +#define DSI_CIO_IRQ_STATEULPS1 (1 << 15) > +#define DSI_CIO_IRQ_STATEULPS2 (1 << 16) > +#define DSI_CIO_IRQ_STATEULPS3 (1 << 17) > +#define DSI_CIO_IRQ_STATEULPS4 (1 << 18) > +#define DSI_CIO_IRQ_STATEULPS5 (1 << 19) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP0_1 (1 << 20) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP1_1 (1 << 21) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP0_2 (1 << 22) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP1_2 (1 << 23) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP0_3 (1 << 24) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP1_3 (1 << 25) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP0_4 (1 << 26) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP1_4 (1 << 27) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP0_5 (1 << 28) > +#define DSI_CIO_IRQ_ERRCONTENTIONLP1_5 (1 << 29) > +#define DSI_CIO_IRQ_ULPSACTIVENOT_ALL0 (1 << 30) > +#define DSI_CIO_IRQ_ULPSACTIVENOT_ALL1 (1 << 31) > +#define DSI_CIO_IRQ_ERROR_MASK \ > + (DSI_CIO_IRQ_ERRSYNCESC1 | DSI_CIO_IRQ_ERRSYNCESC2 | \ > + DSI_CIO_IRQ_ERRSYNCESC3 | DSI_CIO_IRQ_ERRSYNCESC4 | \ > + DSI_CIO_IRQ_ERRSYNCESC5 | \ > + DSI_CIO_IRQ_ERRESC1 | DSI_CIO_IRQ_ERRESC2 | \ > + DSI_CIO_IRQ_ERRESC3 | DSI_CIO_IRQ_ERRESC4 | \ > + DSI_CIO_IRQ_ERRESC5 | \ > + DSI_CIO_IRQ_ERRCONTROL1 | DSI_CIO_IRQ_ERRCONTROL2 | \ > + DSI_CIO_IRQ_ERRCONTROL3 | DSI_CIO_IRQ_ERRCONTROL4 | \ > + DSI_CIO_IRQ_ERRCONTROL5 | \ > + DSI_CIO_IRQ_ERRCONTENTIONLP0_1 | DSI_CIO_IRQ_ERRCONTENTIONLP1_1 | \ > + DSI_CIO_IRQ_ERRCONTENTIONLP0_2 | DSI_CIO_IRQ_ERRCONTENTIONLP1_2 | \ > + DSI_CIO_IRQ_ERRCONTENTIONLP0_3 | DSI_CIO_IRQ_ERRCONTENTIONLP1_3 | \ > + DSI_CIO_IRQ_ERRCONTENTIONLP0_4 | DSI_CIO_IRQ_ERRCONTENTIONLP1_4 | \ > + DSI_CIO_IRQ_ERRCONTENTIONLP0_5 | DSI_CIO_IRQ_ERRCONTENTIONLP1_5) > + > +enum omap_dss_dsi_mode { > + OMAP_DSS_DSI_CMD_MODE = 0, > + OMAP_DSS_DSI_VIDEO_MODE, > +}; > + > +enum omap_dss_dsi_trans_mode { > + /* Sync Pulses: both sync start and end packets sent */ > + OMAP_DSS_DSI_PULSE_MODE, > + /* Sync Events: only sync start packets sent */ > + OMAP_DSS_DSI_EVENT_MODE, > + /* Burst: only sync start packets sent, pixels are time compressed */ > + OMAP_DSS_DSI_BURST_MODE, > +}; > + > +struct omap_dss_dsi_videomode_timings { > + unsigned long hsclk; > + > + unsigned int ndl; > + unsigned int bitspp; > + > + /* pixels */ > + u16 hact; > + /* lines */ > + u16 vact; > + > + /* DSI video mode blanking data */ > + /* Unit: byte clock cycles */ > + u16 hss; > + u16 hsa; > + u16 hse; > + u16 hfp; > + u16 hbp; > + /* Unit: line clocks */ > + u16 vsa; > + u16 vfp; > + u16 vbp; > + > + /* DSI blanking modes */ > + int blanking_mode; > + int hsa_blanking_mode; > + int hbp_blanking_mode; > + int hfp_blanking_mode; > + > + enum omap_dss_dsi_trans_mode trans_mode; > + > + bool ddr_clk_always_on; > + int window_sync; > +}; > + > +struct omap_dss_dsi_config { > + enum omap_dss_dsi_mode mode; > + enum mipi_dsi_pixel_format pixel_format; > + const struct videomode *vm; > + > + unsigned long hs_clk_min, hs_clk_max; > + unsigned long lp_clk_min, lp_clk_max; > + > + bool ddr_clk_always_on; > + enum omap_dss_dsi_trans_mode trans_mode; > +}; > + > +/* DSI PLL HSDIV indices */ > +#define HSDIV_DISPC 0 > +#define HSDIV_DSI 1 > + > +#define DSI_MAX_NR_ISRS 2 > +#define DSI_MAX_NR_LANES 5 > + > +enum dsi_model { > + DSI_MODEL_OMAP3, > + DSI_MODEL_OMAP4, > + DSI_MODEL_OMAP5, > +}; > + > +enum dsi_lane_function { > + DSI_LANE_UNUSED = 0, > + DSI_LANE_CLK, > + DSI_LANE_DATA1, > + DSI_LANE_DATA2, > + DSI_LANE_DATA3, > + DSI_LANE_DATA4, > +}; > + > +struct dsi_lane_config { > + enum dsi_lane_function function; > + u8 polarity; > +}; > + > +typedef void (*omap_dsi_isr_t) (void *arg, u32 mask); > + > +struct dsi_isr_data { > + omap_dsi_isr_t isr; > + void *arg; > + u32 mask; > +}; > + > +enum fifo_size { > + DSI_FIFO_SIZE_0 = 0, > + DSI_FIFO_SIZE_32 = 1, > + DSI_FIFO_SIZE_64 = 2, > + DSI_FIFO_SIZE_96 = 3, > + DSI_FIFO_SIZE_128 = 4, > +}; > + > +enum dsi_vc_source { > + DSI_VC_SOURCE_L4 = 0, > + DSI_VC_SOURCE_VP, > +}; > + > +struct dsi_irq_stats { > + unsigned long last_reset; > + unsigned int irq_count; > + unsigned int dsi_irqs[32]; > + unsigned int vc_irqs[4][32]; > + unsigned int cio_irqs[32]; > +}; > + > +struct dsi_isr_tables { > + struct dsi_isr_data isr_table[DSI_MAX_NR_ISRS]; > + struct dsi_isr_data isr_table_vc[4][DSI_MAX_NR_ISRS]; > + struct dsi_isr_data isr_table_cio[DSI_MAX_NR_ISRS]; > +}; > + > +struct dsi_lp_clock_info { > + unsigned long lp_clk; > + u16 lp_clk_div; > +}; > + > +struct dsi_clk_calc_ctx { > + struct dsi_data *dsi; > + struct dss_pll *pll; > + > + /* inputs */ > + > + const struct omap_dss_dsi_config *config; > + > + unsigned long req_pck_min, req_pck_nom, req_pck_max; > + > + /* outputs */ > + > + struct dss_pll_clock_info dsi_cinfo; > + struct dispc_clock_info dispc_cinfo; > + struct dsi_lp_clock_info lp_cinfo; > + > + struct videomode vm; > + struct omap_dss_dsi_videomode_timings dsi_vm; > +}; > + > +struct dsi_module_id_data { > + u32 address; > + int id; > +}; > + > +enum dsi_quirks { > + DSI_QUIRK_PLL_PWR_BUG = (1 << 0), /* DSI-PLL power command 0x3 is not working */ > + DSI_QUIRK_DCS_CMD_CONFIG_VC = (1 << 1), > + DSI_QUIRK_VC_OCP_WIDTH = (1 << 2), > + DSI_QUIRK_REVERSE_TXCLKESC = (1 << 3), > + DSI_QUIRK_GNQ = (1 << 4), > + DSI_QUIRK_PHY_DCC = (1 << 5), > +}; > + > +struct dsi_of_data { > + enum dsi_model model; > + const struct dss_pll_hw *pll_hw; > + const struct dsi_module_id_data *modules; > + unsigned int max_fck_freq; > + unsigned int max_pll_lpdiv; > + enum dsi_quirks quirks; > +}; > + > +struct dsi_data { > + struct device *dev; > + void __iomem *proto_base; > + void __iomem *phy_base; > + void __iomem *pll_base; > + > + const struct dsi_of_data *data; > + int module_id; > + > + int irq; > + > + bool is_enabled; > + > + struct clk *dss_clk; > + struct regmap *syscon; > + struct dss_device *dss; > + > + struct mipi_dsi_host host; > + > + struct dispc_clock_info user_dispc_cinfo; > + struct dss_pll_clock_info user_dsi_cinfo; > + > + struct dsi_lp_clock_info user_lp_cinfo; > + struct dsi_lp_clock_info current_lp_cinfo; > + > + struct dss_pll pll; > + > + bool vdds_dsi_enabled; > + struct regulator *vdds_dsi_reg; > + > + struct mipi_dsi_device *dsidev; > + > + struct { > + enum dsi_vc_source source; > + enum fifo_size tx_fifo_size; > + enum fifo_size rx_fifo_size; > + } vc[4]; > + > + struct mutex lock; > + struct semaphore bus_lock; > + > + spinlock_t irq_lock; > + struct dsi_isr_tables isr_tables; > + /* space for a copy used by the interrupt handler */ > + struct dsi_isr_tables isr_tables_copy; > + > + int update_vc; > +#ifdef DSI_PERF_MEASURE > + unsigned int update_bytes; > +#endif > + > + /* external TE GPIO */ > + struct gpio_desc *te_gpio; > + int te_irq; > + struct delayed_work te_timeout_work; > + atomic_t do_ext_te_update; > + > + bool te_enabled; > + bool ulps_enabled; > + bool ulps_auto_idle; > + bool video_enabled; > + > + struct delayed_work ulps_work; > + > + struct delayed_work framedone_timeout_work; > + > +#ifdef DSI_CATCH_MISSING_TE > + struct timer_list te_timer; > +#endif > + > + unsigned long cache_req_pck; > + unsigned long cache_clk_freq; > + struct dss_pll_clock_info cache_cinfo; > + > + u32 errors; > + spinlock_t errors_lock; > +#ifdef DSI_PERF_MEASURE > + ktime_t perf_setup_time; > + ktime_t perf_start_time; > +#endif > + int debug_read; > + int debug_write; > + struct { > + struct dss_debugfs_entry *irqs; > + struct dss_debugfs_entry *regs; > + struct dss_debugfs_entry *clks; > + } debugfs; > + > +#ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS > + spinlock_t irq_stats_lock; > + struct dsi_irq_stats irq_stats; > +#endif > + > + unsigned int num_lanes_supported; > + unsigned int line_buffer_size; > + > + struct dsi_lane_config lanes[DSI_MAX_NR_LANES]; > + unsigned int num_lanes_used; > + > + unsigned int scp_clk_refcount; > + > + struct omap_dss_dsi_config config; > + > + struct dss_lcd_mgr_config mgr_config; > + struct videomode vm; > + enum mipi_dsi_pixel_format pix_fmt; > + enum omap_dss_dsi_mode mode; > + struct omap_dss_dsi_videomode_timings vm_timings; > + > + struct omap_dss_device output; > + struct drm_bridge bridge; > +}; > + > +struct dsi_packet_sent_handler_data { > + struct dsi_data *dsi; > + struct completion *completion; > +}; > + > +#endif /* __OMAP_DRM_DSS_DSI_H */ > diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h > index 9df322ca467d..6ecaa060ff4b 100644 > --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h > +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h > @@ -14,7 +14,6 @@ > #include <linux/platform_data/omapdss.h> > > #include <drm/drm_crtc.h> > -#include <drm/drm_mipi_dsi.h> > #include <drm/drm_mode.h> > > #define DISPC_IRQ_FRAMEDONE (1 << 0) > @@ -118,11 +117,6 @@ enum omap_dss_venc_type { > OMAP_DSS_VENC_TYPE_SVIDEO, > }; > > -enum omap_dss_dsi_mode { > - OMAP_DSS_DSI_CMD_MODE = 0, > - OMAP_DSS_DSI_VIDEO_MODE, > -}; > - > enum omap_dss_rotation_type { > OMAP_DSS_ROT_NONE = 0, > OMAP_DSS_ROT_TILER = 1 << 0, > @@ -147,64 +141,6 @@ enum omap_dss_output_id { > OMAP_DSS_OUTPUT_HDMI = 1 << 6, > }; > > -/* DSI */ > - > -enum omap_dss_dsi_trans_mode { > - /* Sync Pulses: both sync start and end packets sent */ > - OMAP_DSS_DSI_PULSE_MODE, > - /* Sync Events: only sync start packets sent */ > - OMAP_DSS_DSI_EVENT_MODE, > - /* Burst: only sync start packets sent, pixels are time compressed */ > - OMAP_DSS_DSI_BURST_MODE, > -}; > - > -struct omap_dss_dsi_videomode_timings { > - unsigned long hsclk; > - > - unsigned int ndl; > - unsigned int bitspp; > - > - /* pixels */ > - u16 hact; > - /* lines */ > - u16 vact; > - > - /* DSI video mode blanking data */ > - /* Unit: byte clock cycles */ > - u16 hss; > - u16 hsa; > - u16 hse; > - u16 hfp; > - u16 hbp; > - /* Unit: line clocks */ > - u16 vsa; > - u16 vfp; > - u16 vbp; > - > - /* DSI blanking modes */ > - int blanking_mode; > - int hsa_blanking_mode; > - int hbp_blanking_mode; > - int hfp_blanking_mode; > - > - enum omap_dss_dsi_trans_mode trans_mode; > - > - bool ddr_clk_always_on; > - int window_sync; > -}; > - > -struct omap_dss_dsi_config { > - enum omap_dss_dsi_mode mode; > - enum mipi_dsi_pixel_format pixel_format; > - const struct videomode *vm; > - > - unsigned long hs_clk_min, hs_clk_max; > - unsigned long lp_clk_min, lp_clk_max; > - > - bool ddr_clk_always_on; > - enum omap_dss_dsi_trans_mode trans_mode; > -}; > - > struct omap_dss_cpr_coefs { > s16 rr, rg, rb; > s16 gr, gg, gb;
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:31PM +0200, Tomi Valkeinen wrote: > We can drop dsi_display_enable(), which just calls > _dsi_display_enable(), and rename _dsi_display_enable() to > dsi_display_enable(). How about adding a comment here to explain why the WARN_ON() is needed anymore ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 16 +++------------- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 4f79d6c664ff..e50418db71ef 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -3738,7 +3738,7 @@ static void dsi_display_uninit_dsi(struct dsi_data *dsi, bool disconnect_lanes, > } > } > > -static void _dsi_display_enable(struct dsi_data *dsi) > +static void dsi_display_enable(struct dsi_data *dsi) > { > int r; > > @@ -3767,16 +3767,6 @@ static void _dsi_display_enable(struct dsi_data *dsi) > DSSDBG("dsi_display_ulps_enable FAILED\n"); > } > > -static void dsi_display_enable(struct omap_dss_device *dssdev) > -{ > - struct dsi_data *dsi = to_dsi_data(dssdev); > - DSSDBG("dsi_display_enable\n"); > - > - WARN_ON(!dsi_bus_is_locked(dsi)); > - > - _dsi_display_enable(dsi); > -} > - > static void _dsi_display_disable(struct dsi_data *dsi, > bool disconnect_lanes, bool enter_ulps) > { > @@ -3851,7 +3841,7 @@ static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable) > return; > > dsi_bus_lock(dsi); > - _dsi_display_enable(dsi); > + dsi_display_enable(dsi); > dsi_enable_te(dsi, true); > dsi_bus_unlock(dsi); > } > @@ -4942,7 +4932,7 @@ static void dsi_bridge_enable(struct drm_bridge *bridge) > > dsi_bus_lock(dsi); > > - dsi_display_enable(dssdev); > + dsi_display_enable(dsi); > > dsi_enable_video_output(dssdev, VC_VIDEO); >
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:33PM +0200, Tomi Valkeinen wrote: > The function names have evolved to be very confusing, and bunch of them > have "display" in them even if the function doesn't deal with display as > such (e.g. dsi_display_enable which just enables the DSI interface). > Rename them by dropping the "display". > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 36 +++++++++++++++---------------- > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index d23fc43f1d1e..ff8ace957291 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -55,8 +55,8 @@ > > static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable); > > -static int dsi_display_init_dispc(struct dsi_data *dsi); > -static void dsi_display_uninit_dispc(struct dsi_data *dsi); > +static int dsi_init_dispc(struct dsi_data *dsi); > +static void dsi_uninit_dispc(struct dsi_data *dsi); > > static int dsi_vc_send_null(struct dsi_data *dsi, int vc, int channel); > > @@ -3257,7 +3257,7 @@ static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc) > u16 word_count; > int r; > > - r = dsi_display_init_dispc(dsi); > + r = dsi_init_dispc(dsi); > if (r) { > dev_err(dsi->dev, "failed to init dispc!\n"); > return; > @@ -3309,7 +3309,7 @@ static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc) > dsi_vc_enable(dsi, vc, false); > } > err_pix_fmt: > - dsi_display_uninit_dispc(dsi); > + dsi_uninit_dispc(dsi); > dev_err(dsi->dev, "failed to enable DSI encoder!\n"); > return; > } > @@ -3331,7 +3331,7 @@ static void dsi_disable_video_output(struct omap_dss_device *dssdev, int vc) > > dss_mgr_disable(&dsi->output); > > - dsi_display_uninit_dispc(dsi); > + dsi_uninit_dispc(dsi); > } > > static void dsi_update_screen_dispc(struct dsi_data *dsi) > @@ -3577,7 +3577,7 @@ static int dsi_configure_dispc_clocks(struct dsi_data *dsi) > return 0; > } > > -static int dsi_display_init_dispc(struct dsi_data *dsi) > +static int dsi_init_dispc(struct dsi_data *dsi) > { > enum omap_channel dispc_channel = dsi->output.dispc_channel; > int r; > @@ -3622,7 +3622,7 @@ static int dsi_display_init_dispc(struct dsi_data *dsi) > return r; > } > > -static void dsi_display_uninit_dispc(struct dsi_data *dsi) > +static void dsi_uninit_dispc(struct dsi_data *dsi) > { > enum omap_channel dispc_channel = dsi->output.dispc_channel; > > @@ -3649,7 +3649,7 @@ static int dsi_configure_dsi_clocks(struct dsi_data *dsi) > return 0; > } > > -static int dsi_display_init_dsi(struct dsi_data *dsi) > +static int dsi_init_dsi(struct dsi_data *dsi) > { > int r; > > @@ -3713,7 +3713,7 @@ static int dsi_display_init_dsi(struct dsi_data *dsi) > return r; > } > > -static void dsi_display_uninit_dsi(struct dsi_data *dsi, bool disconnect_lanes, > +static void dsi_uninit_dsi(struct dsi_data *dsi, bool disconnect_lanes, > bool enter_ulps) > { > if (enter_ulps && !dsi->ulps_enabled) > @@ -3736,7 +3736,7 @@ static void dsi_display_uninit_dsi(struct dsi_data *dsi, bool disconnect_lanes, > } > } > > -static void dsi_display_enable(struct dsi_data *dsi) > +static void dsi_enable(struct dsi_data *dsi) > { > int r; > > @@ -3750,7 +3750,7 @@ static void dsi_display_enable(struct dsi_data *dsi) > > _dsi_initialize_irq(dsi); > > - r = dsi_display_init_dsi(dsi); > + r = dsi_init_dsi(dsi); > if (r) > goto err_init_dsi; > > @@ -3762,10 +3762,10 @@ static void dsi_display_enable(struct dsi_data *dsi) > dsi_runtime_put(dsi); > err_get_dsi: > mutex_unlock(&dsi->lock); > - DSSDBG("dsi_display_ulps_enable FAILED\n"); > + DSSDBG("dsi_enable FAILED\n"); > } > > -static void dsi_display_disable(struct dsi_data *dsi, > +static void dsi_disable(struct dsi_data *dsi, > bool disconnect_lanes, bool enter_ulps) > { > WARN_ON(!dsi_bus_is_locked(dsi)); > @@ -3777,7 +3777,7 @@ static void dsi_display_disable(struct dsi_data *dsi, > dsi_sync_vc(dsi, 2); > dsi_sync_vc(dsi, 3); > > - dsi_display_uninit_dsi(dsi, disconnect_lanes, enter_ulps); > + dsi_uninit_dsi(dsi, disconnect_lanes, enter_ulps); > > dsi_runtime_put(dsi); > > @@ -3807,7 +3807,7 @@ static void omap_dsi_ulps_work_callback(struct work_struct *work) > > dsi_enable_te(dsi, false); > > - dsi_display_disable(dsi, false, true); > + dsi_disable(dsi, false, true); > > dsi_bus_unlock(dsi); > } > @@ -3828,7 +3828,7 @@ static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable) > return; > > dsi_bus_lock(dsi); > - dsi_display_enable(dsi); > + dsi_enable(dsi); > dsi_enable_te(dsi, true); > dsi_bus_unlock(dsi); > } > @@ -4919,7 +4919,7 @@ static void dsi_bridge_enable(struct drm_bridge *bridge) > > dsi_bus_lock(dsi); > > - dsi_display_enable(dsi); > + dsi_enable(dsi); > > dsi_enable_video_output(dssdev, VC_VIDEO); > > @@ -4941,7 +4941,7 @@ static void dsi_bridge_disable(struct drm_bridge *bridge) > > dsi_disable_video_output(dssdev, VC_VIDEO); > > - dsi_display_disable(dsi, true, false); > + dsi_disable(dsi, true, false); > > dsi_bus_unlock(dsi); > }
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:35PM +0200, Tomi Valkeinen wrote: > Clean up the code by separating video-mode enable/disable code into > functions of their own. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 101 +++++++++++++++++------------- > 1 file changed, 57 insertions(+), 44 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 27d0d119668b..6d20245495ac 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -3207,12 +3207,61 @@ static int dsi_configure_pins(struct dsi_data *dsi, > return 0; > } > > -static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc) > +static int dsi_enable_video_mode(struct dsi_data *dsi, int vc) > { > - struct dsi_data *dsi = to_dsi_data(dssdev); > int bpp = mipi_dsi_pixel_format_to_bpp(dsi->pix_fmt); > u8 data_type; > u16 word_count; > + > + switch (dsi->pix_fmt) { > + case MIPI_DSI_FMT_RGB888: > + data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24; > + break; > + case MIPI_DSI_FMT_RGB666: > + data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18; > + break; > + case MIPI_DSI_FMT_RGB666_PACKED: > + data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18; > + break; > + case MIPI_DSI_FMT_RGB565: > + data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16; > + break; > + default: > + return -EINVAL; > + } > + > + dsi_if_enable(dsi, false); > + dsi_vc_enable(dsi, vc, false); > + > + /* MODE, 1 = video mode */ > + REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 1, 4, 4); > + > + word_count = DIV_ROUND_UP(dsi->vm.hactive * bpp, 8); > + > + dsi_vc_write_long_header(dsi, vc, dsi->dsidev->channel, data_type, > + word_count, 0); > + > + dsi_vc_enable(dsi, vc, true); > + dsi_if_enable(dsi, true); > + > + return 0; > +} > + > +static void dsi_disable_video_mode(struct dsi_data *dsi, int vc) > +{ > + dsi_if_enable(dsi, false); > + dsi_vc_enable(dsi, vc, false); > + > + /* MODE, 0 = command mode */ > + REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 0, 4, 4); > + > + dsi_vc_enable(dsi, vc, true); > + dsi_if_enable(dsi, true); > +} > + > +static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc) > +{ > + struct dsi_data *dsi = to_dsi_data(dssdev); > int r; > > r = dsi_init_dispc(dsi); > @@ -3222,37 +3271,9 @@ static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc) > } > > if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE) { > - switch (dsi->pix_fmt) { > - case MIPI_DSI_FMT_RGB888: > - data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24; > - break; > - case MIPI_DSI_FMT_RGB666: > - data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18; > - break; > - case MIPI_DSI_FMT_RGB666_PACKED: > - data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18; > - break; > - case MIPI_DSI_FMT_RGB565: > - data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16; > - break; > - default: > - r = -EINVAL; > - goto err_pix_fmt; > - } > - > - dsi_if_enable(dsi, false); > - dsi_vc_enable(dsi, vc, false); > - > - /* MODE, 1 = video mode */ > - REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 1, 4, 4); > - > - word_count = DIV_ROUND_UP(dsi->vm.hactive * bpp, 8); > - > - dsi_vc_write_long_header(dsi, vc, dsi->dsidev->channel, data_type, > - word_count, 0); > - > - dsi_vc_enable(dsi, vc, true); > - dsi_if_enable(dsi, true); > + r = dsi_enable_video_mode(dsi, vc); > + if (r) > + goto err_video_mode; > } > > r = dss_mgr_enable(&dsi->output); > @@ -3266,7 +3287,7 @@ static void dsi_enable_video_output(struct omap_dss_device *dssdev, int vc) > dsi_if_enable(dsi, false); > dsi_vc_enable(dsi, vc, false); > } > -err_pix_fmt: > +err_video_mode: > dsi_uninit_dispc(dsi); > dev_err(dsi->dev, "failed to enable DSI encoder!\n"); > return; > @@ -3276,16 +3297,8 @@ static void dsi_disable_video_output(struct omap_dss_device *dssdev, int vc) > { > struct dsi_data *dsi = to_dsi_data(dssdev); > > - if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE) { > - dsi_if_enable(dsi, false); > - dsi_vc_enable(dsi, vc, false); > - > - /* MODE, 0 = command mode */ > - REG_FLD_MOD(dsi, DSI_VC_CTRL(vc), 0, 4, 4); > - > - dsi_vc_enable(dsi, vc, true); > - dsi_if_enable(dsi, true); > - } > + if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE) > + dsi_disable_video_mode(dsi, vc); > > dss_mgr_disable(&dsi->output); >
Hi Tomi, Thank you for the patch. On Tue, Nov 24, 2020 at 02:45:37PM +0200, Tomi Valkeinen wrote: > ULPS doesn't work, and I have been unable to get it to work. As ULPS is > a minor power-saving feature which requires substantial amount of > non-trivial code, and we have trouble just getting and > keeping DSI working at all, remove ULPS support. > > When the DSI driver works reliably for command and video mode displays, > someone interested can add it back. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/omapdrm/dss/dsi.c | 297 +----------------------------- > drivers/gpu/drm/omapdrm/dss/dsi.h | 4 - > 2 files changed, 8 insertions(+), 293 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index 6e9c99402540..ffecacd7350a 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -53,8 +53,6 @@ > #define REG_FLD_MOD(dsi, idx, val, start, end) \ > dsi_write_reg(dsi, idx, FLD_MOD(dsi_read_reg(dsi, idx), val, start, end)) > > -static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable); > - > static int dsi_init_dispc(struct dsi_data *dsi); > static void dsi_uninit_dispc(struct dsi_data *dsi); > > @@ -688,44 +686,6 @@ static int dsi_unregister_isr_vc(struct dsi_data *dsi, int vc, > return r; > } > > -static int dsi_register_isr_cio(struct dsi_data *dsi, omap_dsi_isr_t isr, > - void *arg, u32 mask) > -{ > - unsigned long flags; > - int r; > - > - spin_lock_irqsave(&dsi->irq_lock, flags); > - > - r = _dsi_register_isr(isr, arg, mask, dsi->isr_tables.isr_table_cio, > - ARRAY_SIZE(dsi->isr_tables.isr_table_cio)); > - > - if (r == 0) > - _omap_dsi_set_irqs_cio(dsi); > - > - spin_unlock_irqrestore(&dsi->irq_lock, flags); > - > - return r; > -} > - > -static int dsi_unregister_isr_cio(struct dsi_data *dsi, omap_dsi_isr_t isr, > - void *arg, u32 mask) > -{ > - unsigned long flags; > - int r; > - > - spin_lock_irqsave(&dsi->irq_lock, flags); > - > - r = _dsi_unregister_isr(isr, arg, mask, dsi->isr_tables.isr_table_cio, > - ARRAY_SIZE(dsi->isr_tables.isr_table_cio)); > - > - if (r == 0) > - _omap_dsi_set_irqs_cio(dsi); > - > - spin_unlock_irqrestore(&dsi->irq_lock, flags); > - > - return r; > -} > - > static u32 dsi_get_errors(struct dsi_data *dsi) > { > unsigned long flags; > @@ -1450,56 +1410,6 @@ static void dsi_cio_timings(struct dsi_data *dsi) > dsi_write_reg(dsi, DSI_DSIPHY_CFG2, r); > } > > -/* lane masks have lane 0 at lsb. mask_p for positive lines, n for negative */ > -static void dsi_cio_enable_lane_override(struct dsi_data *dsi, > - unsigned int mask_p, > - unsigned int mask_n) > -{ > - int i; > - u32 l; > - u8 lptxscp_start = dsi->num_lanes_supported == 3 ? 22 : 26; > - > - l = 0; > - > - for (i = 0; i < dsi->num_lanes_supported; ++i) { > - unsigned int p = dsi->lanes[i].polarity; > - > - if (mask_p & (1 << i)) > - l |= 1 << (i * 2 + (p ? 0 : 1)); > - > - if (mask_n & (1 << i)) > - l |= 1 << (i * 2 + (p ? 1 : 0)); > - } > - > - /* > - * Bits in REGLPTXSCPDAT4TO0DXDY: > - * 17: DY0 18: DX0 > - * 19: DY1 20: DX1 > - * 21: DY2 22: DX2 > - * 23: DY3 24: DX3 > - * 25: DY4 26: DX4 > - */ > - > - /* Set the lane override configuration */ > - > - /* REGLPTXSCPDAT4TO0DXDY */ > - REG_FLD_MOD(dsi, DSI_DSIPHY_CFG10, l, lptxscp_start, 17); > - > - /* Enable lane override */ > - > - /* ENLPTXSCPDAT */ > - REG_FLD_MOD(dsi, DSI_DSIPHY_CFG10, 1, 27, 27); > -} > - > -static void dsi_cio_disable_lane_override(struct dsi_data *dsi) > -{ > - /* Disable lane override */ > - REG_FLD_MOD(dsi, DSI_DSIPHY_CFG10, 0, 27, 27); /* ENLPTXSCPDAT */ > - /* Reset the lane override configuration */ > - /* REGLPTXSCPDAT4TO0DXDY */ > - REG_FLD_MOD(dsi, DSI_DSIPHY_CFG10, 0, 22, 17); > -} > - > static int dsi_cio_wait_tx_clk_esc_reset(struct dsi_data *dsi) > { > int t, i; > @@ -1674,32 +1584,6 @@ static int dsi_cio_init(struct dsi_data *dsi) > l = FLD_MOD(l, 0x1fff, 12, 0); /* STOP_STATE_COUNTER_IO */ > dsi_write_reg(dsi, DSI_TIMING1, l); > > - if (dsi->ulps_enabled) { > - unsigned int mask_p; > - int i; > - > - DSSDBG("manual ulps exit\n"); > - > - /* ULPS is exited by Mark-1 state for 1ms, followed by > - * stop state. DSS HW cannot do this via the normal > - * ULPS exit sequence, as after reset the DSS HW thinks > - * that we are not in ULPS mode, and refuses to send the > - * sequence. So we need to send the ULPS exit sequence > - * manually by setting positive lines high and negative lines > - * low for 1ms. > - */ > - > - mask_p = 0; > - > - for (i = 0; i < dsi->num_lanes_supported; ++i) { > - if (dsi->lanes[i].function == DSI_LANE_UNUSED) > - continue; > - mask_p |= 1 << i; > - } > - > - dsi_cio_enable_lane_override(dsi, mask_p, 0); > - } > - > r = dsi_cio_power(dsi, DSI_COMPLEXIO_POWER_ON); > if (r) > goto err_cio_pwr; > @@ -1718,17 +1602,6 @@ static int dsi_cio_init(struct dsi_data *dsi) > if (r) > goto err_tx_clk_esc_rst; > > - if (dsi->ulps_enabled) { > - /* Keep Mark-1 state for 1ms (as per DSI spec) */ > - ktime_t wait = ns_to_ktime(1000 * 1000); > - set_current_state(TASK_UNINTERRUPTIBLE); > - schedule_hrtimeout(&wait, HRTIMER_MODE_REL); > - > - /* Disable the override. The lanes should be set to Mark-11 > - * state by the HW */ > - dsi_cio_disable_lane_override(dsi); > - } > - > /* FORCE_TX_STOP_MODE_IO */ > REG_FLD_MOD(dsi, DSI_TIMING1, 0, 15, 15); > > @@ -1739,8 +1612,6 @@ static int dsi_cio_init(struct dsi_data *dsi) > !(dsi->dsidev->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS), > 13, 13); > > - dsi->ulps_enabled = false; > - > DSSDBG("CIO init done\n"); > > return 0; > @@ -1750,8 +1621,6 @@ static int dsi_cio_init(struct dsi_data *dsi) > err_cio_pwr_dom: > dsi_cio_power(dsi, DSI_COMPLEXIO_POWER_OFF); > err_cio_pwr: > - if (dsi->ulps_enabled) > - dsi_cio_disable_lane_override(dsi); > err_scp_clk_dom: > dsi_disable_scp_clk(dsi); > dsi_disable_pads(dsi); > @@ -2522,99 +2391,6 @@ static int dsi_vc_generic_read(struct omap_dss_device *dssdev, int vc, > return r; > } > > -static int dsi_enter_ulps(struct dsi_data *dsi) > -{ > - DECLARE_COMPLETION_ONSTACK(completion); > - int r, i; > - unsigned int mask; > - > - DSSDBG("Entering ULPS"); > - > - WARN_ON(!dsi_bus_is_locked(dsi)); > - > - WARN_ON(dsi->ulps_enabled); > - > - if (dsi->ulps_enabled) > - return 0; > - > - /* DDR_CLK_ALWAYS_ON */ > - if (REG_GET(dsi, DSI_CLK_CTRL, 13, 13)) { > - dsi_if_enable(dsi, 0); > - REG_FLD_MOD(dsi, DSI_CLK_CTRL, 0, 13, 13); > - dsi_if_enable(dsi, 1); > - } > - > - dsi_sync_vc(dsi, 0); > - dsi_sync_vc(dsi, 1); > - dsi_sync_vc(dsi, 2); > - dsi_sync_vc(dsi, 3); > - > - dsi_force_tx_stop_mode_io(dsi); > - > - dsi_vc_enable(dsi, 0, false); > - dsi_vc_enable(dsi, 1, false); > - dsi_vc_enable(dsi, 2, false); > - dsi_vc_enable(dsi, 3, false); > - > - if (REG_GET(dsi, DSI_COMPLEXIO_CFG2, 16, 16)) { /* HS_BUSY */ > - DSSERR("HS busy when enabling ULPS\n"); > - return -EIO; > - } > - > - if (REG_GET(dsi, DSI_COMPLEXIO_CFG2, 17, 17)) { /* LP_BUSY */ > - DSSERR("LP busy when enabling ULPS\n"); > - return -EIO; > - } > - > - r = dsi_register_isr_cio(dsi, dsi_completion_handler, &completion, > - DSI_CIO_IRQ_ULPSACTIVENOT_ALL0); > - if (r) > - return r; > - > - mask = 0; > - > - for (i = 0; i < dsi->num_lanes_supported; ++i) { > - if (dsi->lanes[i].function == DSI_LANE_UNUSED) > - continue; > - mask |= 1 << i; > - } > - /* Assert TxRequestEsc for data lanes and TxUlpsClk for clk lane */ > - /* LANEx_ULPS_SIG2 */ > - REG_FLD_MOD(dsi, DSI_COMPLEXIO_CFG2, mask, 9, 5); > - > - /* flush posted write and wait for SCP interface to finish the write */ > - dsi_read_reg(dsi, DSI_COMPLEXIO_CFG2); > - > - if (wait_for_completion_timeout(&completion, > - msecs_to_jiffies(1000)) == 0) { > - DSSERR("ULPS enable timeout\n"); > - r = -EIO; > - goto err; > - } > - > - dsi_unregister_isr_cio(dsi, dsi_completion_handler, &completion, > - DSI_CIO_IRQ_ULPSACTIVENOT_ALL0); > - > - /* Reset LANEx_ULPS_SIG2 */ > - REG_FLD_MOD(dsi, DSI_COMPLEXIO_CFG2, 0, 9, 5); > - > - /* flush posted write and wait for SCP interface to finish the write */ > - dsi_read_reg(dsi, DSI_COMPLEXIO_CFG2); > - > - dsi_cio_power(dsi, DSI_COMPLEXIO_POWER_ULPS); > - > - dsi_if_enable(dsi, false); > - > - dsi->ulps_enabled = true; > - > - return 0; > - > -err: > - dsi_unregister_isr_cio(dsi, dsi_completion_handler, &completion, > - DSI_CIO_IRQ_ULPSACTIVENOT_ALL0); > - return r; > -} > - > static void dsi_set_lp_rx_timeout(struct dsi_data *dsi, unsigned int ticks, > bool x4, bool x16) > { > @@ -3397,7 +3173,6 @@ static void dsi_handle_framedone(struct dsi_data *dsi, int error) > REG_FLD_MOD(dsi, DSI_TIMING2, 1, 15, 15); /* LP_RX_TO */ > } > > - dsi_set_ulps_auto(dsi, true); > dsi_bus_unlock(dsi); > > if (!error) > @@ -3488,8 +3263,6 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int vc) > > DSSDBG("dsi_update_channel: %d", vc); > > - dsi_set_ulps_auto(dsi, false); > - > r = _dsi_send_nop(dsi, VC_CMD, dsi->dsidev->channel); > if (r < 0) { > DSSWARN("failed to send nop between frames: %d\n", r); > @@ -3509,7 +3282,6 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int vc) > return 0; > > err: > - dsi_set_ulps_auto(dsi, true); > dsi_bus_unlock(dsi); > return r; > } > @@ -3702,12 +3474,8 @@ static int dsi_init_dsi(struct dsi_data *dsi) > return r; > } > > -static void dsi_uninit_dsi(struct dsi_data *dsi, bool disconnect_lanes, > - bool enter_ulps) > +static void dsi_uninit_dsi(struct dsi_data *dsi) > { > - if (enter_ulps && !dsi->ulps_enabled) > - dsi_enter_ulps(dsi); > - > /* disable interface */ > dsi_if_enable(dsi, 0); > dsi_vc_enable(dsi, 0, 0); > @@ -3719,10 +3487,8 @@ static void dsi_uninit_dsi(struct dsi_data *dsi, bool disconnect_lanes, > dsi_cio_uninit(dsi); > dss_pll_disable(&dsi->pll); > > - if (disconnect_lanes) { > - regulator_disable(dsi->vdds_dsi_reg); > - dsi->vdds_dsi_enabled = false; > - } > + regulator_disable(dsi->vdds_dsi_reg); > + dsi->vdds_dsi_enabled = false; > } > > static void dsi_enable(struct dsi_data *dsi) > @@ -3754,8 +3520,7 @@ static void dsi_enable(struct dsi_data *dsi) > DSSDBG("dsi_enable FAILED\n"); > } > > -static void dsi_disable(struct dsi_data *dsi, > - bool disconnect_lanes, bool enter_ulps) > +static void dsi_disable(struct dsi_data *dsi) > { > WARN_ON(!dsi_bus_is_locked(dsi)); > > @@ -3766,7 +3531,7 @@ static void dsi_disable(struct dsi_data *dsi, > dsi_sync_vc(dsi, 2); > dsi_sync_vc(dsi, 3); > > - dsi_uninit_dsi(dsi, disconnect_lanes, enter_ulps); > + dsi_uninit_dsi(dsi); > > dsi_runtime_put(dsi); > > @@ -3787,42 +3552,6 @@ static int dsi_enable_te(struct dsi_data *dsi, bool enable) > return 0; > } > > -static void omap_dsi_ulps_work_callback(struct work_struct *work) > -{ > - struct dsi_data *dsi = container_of(work, struct dsi_data, > - ulps_work.work); > - > - dsi_bus_lock(dsi); > - > - dsi_enable_te(dsi, false); > - > - dsi_disable(dsi, false, true); > - > - dsi_bus_unlock(dsi); > -} > - > -static void dsi_set_ulps_auto(struct dsi_data *dsi, bool enable) > -{ > - WARN_ON(!dsi_bus_is_locked(dsi)); > - > - if (!dsi->ulps_auto_idle) > - return; > - > - if (enable) { > - schedule_delayed_work(&dsi->ulps_work, msecs_to_jiffies(250)); > - } else { > - cancel_delayed_work_sync(&dsi->ulps_work); > - > - if (!dsi->ulps_enabled) > - return; > - > - dsi_bus_lock(dsi); > - dsi_enable(dsi); > - dsi_enable_te(dsi, true); > - dsi_bus_unlock(dsi); > - } > -} > - > #ifdef PRINT_VERBOSE_VM_TIMINGS > static void print_dsi_vm(const char *str, > const struct omap_dss_dsi_videomode_timings *t) > @@ -4494,13 +4223,10 @@ static ssize_t omap_dsi_host_transfer(struct mipi_dsi_host *host, > > dsi_bus_lock(dsi); > > - if (dsi->video_enabled) { > - dsi_set_ulps_auto(dsi, false); > + if (dsi->video_enabled) > r = _omap_dsi_host_transfer(dsi, vc, msg); > - dsi_set_ulps_auto(dsi, true); > - } else { > + else > r = -EIO; > - } > > dsi_bus_unlock(dsi); > > @@ -4642,9 +4368,6 @@ static int omap_dsi_host_attach(struct mipi_dsi_host *host, > dsi->dsidev = client; > dsi->pix_fmt = client->format; > > - INIT_DEFERRABLE_WORK(&dsi->ulps_work, > - omap_dsi_ulps_work_callback); > - > dsi->config.hs_clk_min = 150000000; // TODO: get from client? > dsi->config.hs_clk_max = client->hs_rate; > dsi->config.lp_clk_min = 7000000; // TODO: get from client? > @@ -4657,8 +4380,6 @@ static int omap_dsi_host_attach(struct mipi_dsi_host *host, > else > dsi->config.trans_mode = OMAP_DSS_DSI_EVENT_MODE; > > - dsi->ulps_auto_idle = false; > - > return 0; > } > > @@ -4913,8 +4634,6 @@ static void dsi_bridge_enable(struct drm_bridge *bridge) > > dsi->video_enabled = true; > > - dsi_set_ulps_auto(dsi, true); > - > dsi_bus_unlock(dsi); > } > > @@ -4929,7 +4648,7 @@ static void dsi_bridge_disable(struct drm_bridge *bridge) > > dsi_disable_video_output(dssdev, VC_VIDEO); > > - dsi_disable(dsi, true, false); > + dsi_disable(dsi); > > dsi_bus_unlock(dsi); > } > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.h b/drivers/gpu/drm/omapdrm/dss/dsi.h > index 3543828e30eb..452cee3279db 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.h > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.h > @@ -391,12 +391,8 @@ struct dsi_data { > atomic_t do_ext_te_update; > > bool te_enabled; > - bool ulps_enabled; > - bool ulps_auto_idle; > bool video_enabled; > > - struct delayed_work ulps_work; > - > struct delayed_work framedone_timeout_work; > > #ifdef DSI_CATCH_MISSING_TE
* Tomi Valkeinen <tomi.valkeinen@ti.com> [201124 12:48]: > From: Sebastian Reichel <sebastian.reichel@collabora.com> > > The DSI command mode panel is no longer specific > to OMAP and thus the config option has been renamed > slightly. > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Tony Lindgren <tony@atomide.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Best to merge this along with the dss patches: Acked-by: Tony Lindgren <tony@atomide.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [201201 10:39]: > On 30/11/2020 11:53, Laurent Pinchart wrote: > > Hi Tomi, > > > > Thank you for the patch. > > > > On Tue, Nov 24, 2020 at 02:45:15PM +0200, Tomi Valkeinen wrote: > >> Add address-cells & size-cells to DSI nodes so that board files do not > >> need to define them. > > > > How about adding ports too, while at it ? > > We don't have ports for other encoders either. I'm not sure if adding ports helps or not, but I > think it makes sense to add them consistently to all encoders on all boards if we want to go that way. > > > It would also be nice to convert the DT bindings to YAML :-) > > I agree, but not as part of this already 81 patch series. =) Please feel free to merge this via the dss tree: Acked-by: Tony Lindgren <tony@atomide.com>
On 01/12/2020 02:27, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Tue, Nov 24, 2020 at 02:45:28PM +0200, Tomi Valkeinen wrote: >> Drop unneeded includes. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/gpu/drm/panel/panel-dsi-cm.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c >> index ec87b785871f..91ed8237a1c2 100644 >> --- a/drivers/gpu/drm/panel/panel-dsi-cm.c >> +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c >> @@ -9,12 +9,7 @@ >> #include <linux/backlight.h> >> #include <linux/delay.h> >> #include <linux/gpio/consumer.h> >> -#include <linux/interrupt.h> > > This could go to the patch that moves TE handling to the code. True. >> #include <linux/jiffies.h> >> -#include <linux/module.h> > > I'd keep module.h as you use macros it defines, and we shouldn't depend > in indirect includes. Ok. Tomi
Hi, On Tue, Nov 24, 2020 at 02:45:12PM +0200, Tomi Valkeinen wrote: > dss_mgr_ops was needed with the multi-module architecture, but is no > longer needed. We can thus remove it and use direct calls. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/gpu/drm/omapdrm/dss/dss.h | 1 - > drivers/gpu/drm/omapdrm/dss/omapdss.h | 42 +++++++++++---------------- > drivers/gpu/drm/omapdrm/dss/output.c | 33 +++++---------------- > drivers/gpu/drm/omapdrm/omap_crtc.c | 38 +++++------------------- > drivers/gpu/drm/omapdrm/omap_crtc.h | 2 -- > drivers/gpu/drm/omapdrm/omap_drv.c | 4 +-- > 6 files changed, 33 insertions(+), 87 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dss.h b/drivers/gpu/drm/omapdrm/dss/dss.h > index 96f702314c8c..a547527bb2f3 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dss.h > +++ b/drivers/gpu/drm/omapdrm/dss/dss.h > @@ -257,7 +257,6 @@ struct dss_device { > struct dss_pll *video2_pll; > > struct dispc_device *dispc; > - const struct dss_mgr_ops *mgr_ops; > struct omap_drm_private *mgr_ops_priv; > }; > > diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h > index fba5f05e5e48..9df322ca467d 100644 > --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h > +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h > @@ -338,31 +338,23 @@ enum dss_writeback_channel { > DSS_WB_LCD3_MGR = 7, > }; > > -struct dss_mgr_ops { > - void (*start_update)(struct omap_drm_private *priv, > - enum omap_channel channel); > - int (*enable)(struct omap_drm_private *priv, > - enum omap_channel channel); > - void (*disable)(struct omap_drm_private *priv, > - enum omap_channel channel); > - void (*set_timings)(struct omap_drm_private *priv, > - enum omap_channel channel, > - const struct videomode *vm); > - void (*set_lcd_config)(struct omap_drm_private *priv, > - enum omap_channel channel, > - const struct dss_lcd_mgr_config *config); > - int (*register_framedone_handler)(struct omap_drm_private *priv, > - enum omap_channel channel, > - void (*handler)(void *), void *data); > - void (*unregister_framedone_handler)(struct omap_drm_private *priv, > - enum omap_channel channel, > - void (*handler)(void *), void *data); > -}; > - > -int dss_install_mgr_ops(struct dss_device *dss, > - const struct dss_mgr_ops *mgr_ops, > - struct omap_drm_private *priv); > -void dss_uninstall_mgr_ops(struct dss_device *dss); > +void omap_crtc_dss_start_update(struct omap_drm_private *priv, > + enum omap_channel channel); > +void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable); > +int omap_crtc_dss_enable(struct omap_drm_private *priv, enum omap_channel channel); > +void omap_crtc_dss_disable(struct omap_drm_private *priv, enum omap_channel channel); > +void omap_crtc_dss_set_timings(struct omap_drm_private *priv, > + enum omap_channel channel, > + const struct videomode *vm); > +void omap_crtc_dss_set_lcd_config(struct omap_drm_private *priv, > + enum omap_channel channel, > + const struct dss_lcd_mgr_config *config); > +int omap_crtc_dss_register_framedone( > + struct omap_drm_private *priv, enum omap_channel channel, > + void (*handler)(void *), void *data); > +void omap_crtc_dss_unregister_framedone( > + struct omap_drm_private *priv, enum omap_channel channel, > + void (*handler)(void *), void *data); > > void dss_mgr_set_timings(struct omap_dss_device *dssdev, > const struct videomode *vm); > diff --git a/drivers/gpu/drm/omapdrm/dss/output.c b/drivers/gpu/drm/omapdrm/dss/output.c > index 7a14d2b5b2f7..2121c947947b 100644 > --- a/drivers/gpu/drm/omapdrm/dss/output.c > +++ b/drivers/gpu/drm/omapdrm/dss/output.c > @@ -81,54 +81,35 @@ void omapdss_device_cleanup_output(struct omap_dss_device *out) > out->next_bridge : out->bridge); > } > > -int dss_install_mgr_ops(struct dss_device *dss, > - const struct dss_mgr_ops *mgr_ops, > - struct omap_drm_private *priv) > -{ > - if (dss->mgr_ops) > - return -EBUSY; > - > - dss->mgr_ops = mgr_ops; > - dss->mgr_ops_priv = priv; > - > - return 0; > -} > - > -void dss_uninstall_mgr_ops(struct dss_device *dss) > -{ > - dss->mgr_ops = NULL; > - dss->mgr_ops_priv = NULL; > -} > - > void dss_mgr_set_timings(struct omap_dss_device *dssdev, > const struct videomode *vm) > { > - dssdev->dss->mgr_ops->set_timings(dssdev->dss->mgr_ops_priv, > + omap_crtc_dss_set_timings(dssdev->dss->mgr_ops_priv, > dssdev->dispc_channel, vm); > } > > void dss_mgr_set_lcd_config(struct omap_dss_device *dssdev, > const struct dss_lcd_mgr_config *config) > { > - dssdev->dss->mgr_ops->set_lcd_config(dssdev->dss->mgr_ops_priv, > + omap_crtc_dss_set_lcd_config(dssdev->dss->mgr_ops_priv, > dssdev->dispc_channel, config); > } > > int dss_mgr_enable(struct omap_dss_device *dssdev) > { > - return dssdev->dss->mgr_ops->enable(dssdev->dss->mgr_ops_priv, > + return omap_crtc_dss_enable(dssdev->dss->mgr_ops_priv, > dssdev->dispc_channel); > } > > void dss_mgr_disable(struct omap_dss_device *dssdev) > { > - dssdev->dss->mgr_ops->disable(dssdev->dss->mgr_ops_priv, > + omap_crtc_dss_disable(dssdev->dss->mgr_ops_priv, > dssdev->dispc_channel); > } > > void dss_mgr_start_update(struct omap_dss_device *dssdev) > { > - dssdev->dss->mgr_ops->start_update(dssdev->dss->mgr_ops_priv, > + omap_crtc_dss_start_update(dssdev->dss->mgr_ops_priv, > dssdev->dispc_channel); > } > > @@ -137,7 +118,7 @@ int dss_mgr_register_framedone_handler(struct omap_dss_device *dssdev, > { > struct dss_device *dss = dssdev->dss; > > - return dss->mgr_ops->register_framedone_handler(dss->mgr_ops_priv, > + return omap_crtc_dss_register_framedone(dss->mgr_ops_priv, > dssdev->dispc_channel, > handler, data); > } > @@ -147,7 +128,7 @@ void dss_mgr_unregister_framedone_handler(struct omap_dss_device *dssdev, > { > struct dss_device *dss = dssdev->dss; > > - dss->mgr_ops->unregister_framedone_handler(dss->mgr_ops_priv, > + omap_crtc_dss_unregister_framedone(dss->mgr_ops_priv, > dssdev->dispc_channel, > handler, data); > } > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c > index 0bf5cef579b5..e3259338afb9 100644 > --- a/drivers/gpu/drm/omapdrm/omap_crtc.c > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c > @@ -100,14 +100,14 @@ int omap_crtc_wait_pending(struct drm_crtc *crtc) > * the upstream part of the video pipe. > */ > > -static void omap_crtc_dss_start_update(struct omap_drm_private *priv, > +void omap_crtc_dss_start_update(struct omap_drm_private *priv, > enum omap_channel channel) > { > dispc_mgr_enable(priv->dispc, channel, true); > } > > /* Called only from the encoder enable/disable and suspend/resume handlers. */ > -static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable) > +void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable) > { > struct omap_crtc_state *omap_state = to_omap_crtc_state(crtc->state); > struct drm_device *dev = crtc->dev; > @@ -180,8 +180,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable) > } > > > -static int omap_crtc_dss_enable(struct omap_drm_private *priv, > - enum omap_channel channel) > +int omap_crtc_dss_enable(struct omap_drm_private *priv, enum omap_channel channel) > { > struct drm_crtc *crtc = priv->channels[channel]->crtc; > struct omap_crtc *omap_crtc = to_omap_crtc(crtc); > @@ -193,8 +192,7 @@ static int omap_crtc_dss_enable(struct omap_drm_private *priv, > return 0; > } > > -static void omap_crtc_dss_disable(struct omap_drm_private *priv, > - enum omap_channel channel) > +void omap_crtc_dss_disable(struct omap_drm_private *priv, enum omap_channel channel) > { > struct drm_crtc *crtc = priv->channels[channel]->crtc; > struct omap_crtc *omap_crtc = to_omap_crtc(crtc); > @@ -202,7 +200,7 @@ static void omap_crtc_dss_disable(struct omap_drm_private *priv, > omap_crtc_set_enabled(&omap_crtc->base, false); > } > > -static void omap_crtc_dss_set_timings(struct omap_drm_private *priv, > +void omap_crtc_dss_set_timings(struct omap_drm_private *priv, > enum omap_channel channel, > const struct videomode *vm) > { > @@ -213,7 +211,7 @@ static void omap_crtc_dss_set_timings(struct omap_drm_private *priv, > omap_crtc->vm = *vm; > } > > -static void omap_crtc_dss_set_lcd_config(struct omap_drm_private *priv, > +void omap_crtc_dss_set_lcd_config(struct omap_drm_private *priv, > enum omap_channel channel, > const struct dss_lcd_mgr_config *config) > { > @@ -225,7 +223,7 @@ static void omap_crtc_dss_set_lcd_config(struct omap_drm_private *priv, > config); > } > > -static int omap_crtc_dss_register_framedone( > +int omap_crtc_dss_register_framedone( > struct omap_drm_private *priv, enum omap_channel channel, > void (*handler)(void *), void *data) > { > @@ -244,7 +242,7 @@ static int omap_crtc_dss_register_framedone( > return 0; > } > > -static void omap_crtc_dss_unregister_framedone( > +void omap_crtc_dss_unregister_framedone( > struct omap_drm_private *priv, enum omap_channel channel, > void (*handler)(void *), void *data) > { > @@ -261,16 +259,6 @@ static void omap_crtc_dss_unregister_framedone( > omap_crtc->framedone_handler_data = NULL; > } > > -static const struct dss_mgr_ops mgr_ops = { > - .start_update = omap_crtc_dss_start_update, > - .enable = omap_crtc_dss_enable, > - .disable = omap_crtc_dss_disable, > - .set_timings = omap_crtc_dss_set_timings, > - .set_lcd_config = omap_crtc_dss_set_lcd_config, > - .register_framedone_handler = omap_crtc_dss_register_framedone, > - .unregister_framedone_handler = omap_crtc_dss_unregister_framedone, > -}; > - > /* ----------------------------------------------------------------------------- > * Setup, Flush and Page Flip > */ > @@ -753,16 +741,6 @@ static const char *channel_names[] = { > [OMAP_DSS_CHANNEL_LCD3] = "lcd3", > }; > > -void omap_crtc_pre_init(struct omap_drm_private *priv) > -{ > - dss_install_mgr_ops(priv->dss, &mgr_ops, priv); > -} > - > -void omap_crtc_pre_uninit(struct omap_drm_private *priv) > -{ > - dss_uninstall_mgr_ops(priv->dss); > -} > - > /* initialize crtc */ > struct drm_crtc *omap_crtc_init(struct drm_device *dev, > struct omap_drm_pipeline *pipe, > diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.h b/drivers/gpu/drm/omapdrm/omap_crtc.h > index 2fd57751ae2b..a8b9cbee86e0 100644 > --- a/drivers/gpu/drm/omapdrm/omap_crtc.h > +++ b/drivers/gpu/drm/omapdrm/omap_crtc.h > @@ -22,8 +22,6 @@ struct videomode; > > struct videomode *omap_crtc_timings(struct drm_crtc *crtc); > enum omap_channel omap_crtc_channel(struct drm_crtc *crtc); > -void omap_crtc_pre_init(struct omap_drm_private *priv); > -void omap_crtc_pre_uninit(struct omap_drm_private *priv); > struct drm_crtc *omap_crtc_init(struct drm_device *dev, > struct omap_drm_pipeline *pipe, > struct drm_plane *plane); > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c > index 46cb088c2977..da7183e98e3f 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.c > +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > @@ -581,7 +581,7 @@ static int omapdrm_init(struct omap_drm_private *priv, struct device *dev) > priv->dss = pdata->dss; > priv->dispc = dispc_get_dispc(priv->dss); > > - omap_crtc_pre_init(priv); > + priv->dss->mgr_ops_priv = priv; > > soc = soc_device_match(omapdrm_soc_devices); > priv->omaprev = soc ? (unsigned int)soc->data : 0; > @@ -634,7 +634,6 @@ static int omapdrm_init(struct omap_drm_private *priv, struct device *dev) > omap_gem_deinit(ddev); > destroy_workqueue(priv->wq); > omap_disconnect_pipelines(ddev); > - omap_crtc_pre_uninit(priv); > drm_dev_put(ddev); > return ret; > } > @@ -660,7 +659,6 @@ static void omapdrm_cleanup(struct omap_drm_private *priv) > destroy_workqueue(priv->wq); > > omap_disconnect_pipelines(ddev); > - omap_crtc_pre_uninit(priv); > > drm_dev_put(ddev); > } > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >
Hi, On Tue, Nov 24, 2020 at 02:45:13PM +0200, Tomi Valkeinen wrote: > Use the common MIPI_DCS_GET_ERROR_COUNT_ON_DSI define instead of > driver's own. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/gpu/drm/panel/panel-dsi-cm.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-dsi-cm.c b/drivers/gpu/drm/panel/panel-dsi-cm.c > index 35a0c7da1974..cb0d27a38555 100644 > --- a/drivers/gpu/drm/panel/panel-dsi-cm.c > +++ b/drivers/gpu/drm/panel/panel-dsi-cm.c > @@ -27,7 +27,6 @@ > #include <video/of_display_timing.h> > #include <video/videomode.h> > > -#define DCS_READ_NUM_ERRORS 0x05 > #define DCS_GET_ID1 0xda > #define DCS_GET_ID2 0xdb > #define DCS_GET_ID3 0xdc > @@ -225,7 +224,7 @@ static ssize_t num_dsi_errors_show(struct device *dev, > mutex_lock(&ddata->lock); > > if (ddata->enabled) > - r = dsicm_dcs_read_1(ddata, DCS_READ_NUM_ERRORS, &errors); > + r = dsicm_dcs_read_1(ddata, MIPI_DCS_GET_ERROR_COUNT_ON_DSI, &errors); > > mutex_unlock(&ddata->lock); > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >
Hi, On Tue, Nov 24, 2020 at 02:45:15PM +0200, Tomi Valkeinen wrote: > Add address-cells & size-cells to DSI nodes so that board files do not > need to define them. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Tony Lindgren <tony@atomide.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > arch/arm/boot/dts/omap5.dtsi | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi > index 2bf2e5839a7f..e6f6947965ef 100644 > --- a/arch/arm/boot/dts/omap5.dtsi > +++ b/arch/arm/boot/dts/omap5.dtsi > @@ -517,6 +517,9 @@ dsi1: encoder@0 { > clocks = <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 8>, > <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>; > clock-names = "fck", "sys_clk"; > + > + #address-cells = <1>; > + #size-cells = <0>; > }; > }; > > @@ -549,6 +552,9 @@ dsi2: encoder@0 { > clocks = <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 8>, > <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>; > clock-names = "fck", "sys_clk"; > + > + #address-cells = <1>; > + #size-cells = <0>; > }; > }; > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >
Hi, On Tue, Nov 24, 2020 at 02:45:17PM +0200, Tomi Valkeinen wrote: > The DSI host driver currently ignores the video mode flags in > client->mode_flags. Add the code to take the transfer mode from client's > mode_flags. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > drivers/gpu/drm/omapdrm/dss/dsi.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c > index c3592c6db977..7fee9cf8782d 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dsi.c > +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c > @@ -5140,6 +5140,13 @@ static int omap_dsi_host_attach(struct mipi_dsi_host *host, > dsi->config.lp_clk_min = 7000000; // TODO: get from client? > dsi->config.lp_clk_max = client->lp_rate; > > + if (client->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) > + dsi->config.trans_mode = OMAP_DSS_DSI_BURST_MODE; > + else if (client->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) > + dsi->config.trans_mode = OMAP_DSS_DSI_PULSE_MODE; > + else > + dsi->config.trans_mode = OMAP_DSS_DSI_EVENT_MODE; > + > dsi->ulps_auto_idle = false; > > return 0; > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >
On Tue, 24 Nov 2020 04:44:57 -0800, Tomi Valkeinen wrote: > > From: Sebastian Reichel <sebastian.reichel@collabora.com> > > The panel driver is no longer using any OMAP specific APIs, so > let's move it into the generic panel directory. > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Sam Ravnborg <sam@ravnborg.org> > Acked-by: Sam Ravnborg <sam@ravnborg.org> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/omapdrm/Kconfig | 1 - > drivers/gpu/drm/omapdrm/Makefile | 1 - > drivers/gpu/drm/omapdrm/displays/Kconfig | 10 ---------- > drivers/gpu/drm/omapdrm/displays/Makefile | 2 -- > drivers/gpu/drm/panel/Kconfig | 9 +++++++++ > drivers/gpu/drm/panel/Makefile | 1 + > .../gpu/drm/{omapdrm/displays => panel}/panel-dsi-cm.c | 0 Not sure if it's a result of this commit but on drm-tip we see: $ make allmodconfig $ make modules_check DESCEND objtool CALL scripts/atomic/check-atomics.sh CALL scripts/checksyscalls.sh CHK include/generated/compile.h error: the following would cause module name conflict: drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.ko drivers/gpu/drm/panel/panel-dsi-cm.ko make: *** [Makefile:1400: modules_check] Error 1
Hi, On Fri, Jan 08, 2021 at 01:23:54PM -0800, Dixit, Ashutosh wrote: > On Tue, 24 Nov 2020 04:44:57 -0800, Tomi Valkeinen wrote: > > > > From: Sebastian Reichel <sebastian.reichel@collabora.com> > > > > The panel driver is no longer using any OMAP specific APIs, so > > let's move it into the generic panel directory. > > > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > > Cc: Thierry Reding <thierry.reding@gmail.com> > > Cc: Sam Ravnborg <sam@ravnborg.org> > > Acked-by: Sam Ravnborg <sam@ravnborg.org> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > drivers/gpu/drm/omapdrm/Kconfig | 1 - > > drivers/gpu/drm/omapdrm/Makefile | 1 - > > drivers/gpu/drm/omapdrm/displays/Kconfig | 10 ---------- > > drivers/gpu/drm/omapdrm/displays/Makefile | 2 -- > > drivers/gpu/drm/panel/Kconfig | 9 +++++++++ > > drivers/gpu/drm/panel/Makefile | 1 + > > .../gpu/drm/{omapdrm/displays => panel}/panel-dsi-cm.c | 0 > > Not sure if it's a result of this commit but on drm-tip we see: > > $ make allmodconfig > $ make modules_check > DESCEND objtool > CALL scripts/atomic/check-atomics.sh > CALL scripts/checksyscalls.sh > CHK include/generated/compile.h > error: the following would cause module name conflict: > drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.ko > drivers/gpu/drm/panel/panel-dsi-cm.ko > make: *** [Makefile:1400: modules_check] Error 1 It is a result of this commit and it has already been reported by Stephen Rothwell. The thread also contains a patch to fixup the problem: https://lore.kernel.org/linux-next/20210108195839.GA1429715@ravnborg.org/T/#m0eee5e806cc93cf9982620b7b8b9f77df2c37498 Sorry for the inconvenience, -- Sebastian
Hi, v4 of the omapdrm DSI series. Link to v3: https://www.spinics.net/lists/linux-omap/msg157082.html There are a lot of changes since v3, but most of them should be in new patches on top. The main problem has been video mode panels, which are still not quite working with this series. However, I have pinpointed the issues quite well (I think), and I have a few small hacks which gets a video mode panel working (the single one I have). The problem is with the sequence the dsi host and the panel are initialized, and I did not figure out how to solve that without adding a new bridge callback (post_enable). However, the issue could also be panel specific, omap dsi driver bug, or panel driver bug. This needs more study and work, but as we don't have any video mode users in upstream, I think the work can be done on top of this series. This series, and the hacks (along with a few other hacks) can be found from: git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git 5.11/dsi Tested with OMAP4 SDP (command mode panel) and OMAP5 uEVM with a custom video mode panel setup (with a broken cable which only works in certain position and when the planets are aligned). Changes in v4: - Resolved the comments received in v3, and added the tags - Lots of new patches continuing the cleanup work. Most of these do not help with the video mode as such, but as the issues have been very difficult to debug, I've been doing cleanups along the way so that I can find the problems more easily. - Dropped ULPS. Complex feature, doesn't work, would give little benefit. Tomi Sebastian Reichel (49): Revert "drm/omap: dss: Remove unused omap_dss_device operations" drm/omap: drop unused dsi.configure_pins drm/omap: dsi: use MIPI_DSI_FMT_* instead of OMAP_DSS_DSI_FMT_* drm/omap: constify write buffers drm/omap: dsi: add generic transfer function drm/omap: panel-dsi-cm: convert to transfer API drm/omap: dsi: unexport specific data transfer functions drm/omap: dsi: drop virtual channel logic drm/omap: dsi: simplify write function drm/omap: dsi: simplify read functions drm/omap: dsi: switch dsi_vc_send_long/short to mipi_dsi_msg drm/omap: dsi: introduce mipi_dsi_host drm/omap: panel-dsi-cm: use DSI helpers drm/omap: dsi: request VC via mipi_dsi_attach drm/omap: panel-dsi-cm: drop hardcoded VC drm/omap: panel-dsi-cm: use common MIPI DCS 1.3 defines drm/omap: dsi: drop unused memory_read() drm/omap: dsi: drop unused get_te() drm/omap: dsi: drop unused enable_te() drm/omap: dsi: drop useless sync() drm/omap: dsi: use pixel-format and mode from attach drm/omap: panel-dsi-cm: use bulk regulator API drm/omap: dsi: lp/hs switching support for transfer() drm/omap: dsi: move TE GPIO handling into core drm/omap: dsi: drop custom enable_te() API drm/omap: dsi: do bus locking in host driver drm/omap: dsi: untangle ulps ops from enable/disable drm/omap: dsi: do ULPS in host driver drm/omap: dsi: move panel refresh function to host drm/omap: dsi: Reverse direction of the DSS device enable/disable operations drm/omap: dsi: drop custom panel capability support drm/omap: dsi: convert to drm_panel drm/omap: drop omapdss-boot-init drm/omap: dsi: implement check timings drm/omap: panel-dsi-cm: use DEVICE_ATTR_RO drm/omap: panel-dsi-cm: support unbinding drm/omap: panel-dsi-cm: fix remove() drm/omap: remove global dss_device variable drm/panel: Move OMAP's DSI command mode panel driver drm/omap: dsi: Register a drm_bridge drm/omap: remove legacy DSS device operations drm/omap: remove unused omap_connector drm/omap: simplify omap_display_id drm/omap: drop unused DSS next pointer drm/omap: drop DSS ops_flags drm/omap: drop dssdev display field drm/omap: simplify DSI manual update code drm/omap: dsi: simplify pin config ARM: omap2plus_defconfig: Update for moved DSI command mode panel Tomi Valkeinen (31): drm/omap: squash omapdrm sub-modules into one drm/omap: remove unused display.c drm/omap: drop unused owner field drm/omap: remove dispc_ops drm/omap: remove dss_mgr_ops drm/panel: panel-dsi-cm: use MIPI_DCS_GET_ERROR_COUNT_ON_DSI drm/panel: panel-dsi-cm: cleanup tear enable ARM: dts: omap5: add address-cells & size-cells to dsi drm/omap: pll: fix iteration loop check drm/omap: dsi: set trans_mode according to client mode_flags drm/panel: panel-dsi-cm: set column & page at setup drm/omap: dsi: send nop instead of page & column drm/omap: dsi: simplify VC handling drm/omap: dsi: drop useless channel checks drm/omap: dsi: cleanup channel usages drm/omap: dsi: skip dsi_vc_enable_hs when already in correct mode drm/omap: dsi: set LP/HS before update drm/omap: dsi: use separate VCs for cmd and video drm/panel: panel-dsi-cm: remove extra 'if' drm/panel: panel-dsi-cm: add panel database to driver drm/panel: panel-dsi-cm: drop unneeded includes drm/omap: dsi: move structs & defines to dsi.h drm/omap: dsi: move enable/disable to bridge enable/disable drm/omap: dsi: display_enable cleanup drm/omap: dsi: display_disable cleanup drm/omap: dsi: rename dsi_display_* functions drm/omap: dsi: cleanup initial vc setup drm/omap: dsi: split video mode enable/disable into separate func drm/omap: dsi: fix and cleanup ddr_clk_always_on drm/omap: dsi: remove ulps support drm/omap: dsi: fix DCS_CMD_ENABLE arch/arm/boot/dts/omap5.dtsi | 6 + arch/arm/configs/omap2plus_defconfig | 2 +- drivers/gpu/drm/omapdrm/Kconfig | 120 +- drivers/gpu/drm/omapdrm/Makefile | 19 +- drivers/gpu/drm/omapdrm/displays/Kconfig | 10 - drivers/gpu/drm/omapdrm/displays/Makefile | 2 - .../gpu/drm/omapdrm/displays/panel-dsi-cm.c | 1385 ------------ drivers/gpu/drm/omapdrm/dss/Kconfig | 135 -- drivers/gpu/drm/omapdrm/dss/Makefile | 20 - drivers/gpu/drm/omapdrm/dss/base.c | 87 +- drivers/gpu/drm/omapdrm/dss/dispc.c | 101 +- drivers/gpu/drm/omapdrm/dss/display.c | 60 - drivers/gpu/drm/omapdrm/dss/dpi.c | 1 - drivers/gpu/drm/omapdrm/dss/dsi.c | 1905 +++++++---------- drivers/gpu/drm/omapdrm/dss/dsi.h | 450 ++++ drivers/gpu/drm/omapdrm/dss/dss.c | 28 +- drivers/gpu/drm/omapdrm/dss/dss.h | 72 +- drivers/gpu/drm/omapdrm/dss/hdmi4.c | 1 - drivers/gpu/drm/omapdrm/dss/hdmi5.c | 1 - .../gpu/drm/omapdrm/dss/omapdss-boot-init.c | 229 -- drivers/gpu/drm/omapdrm/dss/omapdss.h | 338 +-- drivers/gpu/drm/omapdrm/dss/output.c | 57 +- drivers/gpu/drm/omapdrm/dss/pll.c | 6 + drivers/gpu/drm/omapdrm/dss/sdi.c | 1 - drivers/gpu/drm/omapdrm/dss/venc.c | 2 - drivers/gpu/drm/omapdrm/omap_connector.c | 157 -- drivers/gpu/drm/omapdrm/omap_connector.h | 28 - drivers/gpu/drm/omapdrm/omap_crtc.c | 103 +- drivers/gpu/drm/omapdrm/omap_crtc.h | 2 - drivers/gpu/drm/omapdrm/omap_drv.c | 73 +- drivers/gpu/drm/omapdrm/omap_drv.h | 3 +- drivers/gpu/drm/omapdrm/omap_encoder.c | 59 +- drivers/gpu/drm/omapdrm/omap_irq.c | 34 +- drivers/gpu/drm/omapdrm/omap_plane.c | 12 +- drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-dsi-cm.c | 670 ++++++ 37 files changed, 2249 insertions(+), 3940 deletions(-) delete mode 100644 drivers/gpu/drm/omapdrm/displays/Kconfig delete mode 100644 drivers/gpu/drm/omapdrm/displays/Makefile delete mode 100644 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c delete mode 100644 drivers/gpu/drm/omapdrm/dss/Kconfig delete mode 100644 drivers/gpu/drm/omapdrm/dss/Makefile delete mode 100644 drivers/gpu/drm/omapdrm/dss/display.c create mode 100644 drivers/gpu/drm/omapdrm/dss/dsi.h delete mode 100644 drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c delete mode 100644 drivers/gpu/drm/omapdrm/omap_connector.c delete mode 100644 drivers/gpu/drm/omapdrm/omap_connector.h create mode 100644 drivers/gpu/drm/panel/panel-dsi-cm.c -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki