Message ID | 20201124124538.660710-56-tomi.valkeinen@ti.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Convert DSI code to use drm_mipi_dsi and drm_panel | expand |
Hi Tomi, 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. Sam
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
Hi Tomi On Tue, Nov 24, 2020 at 06:26:47PM +0200, Tomi Valkeinen wrote: > 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. 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. Sam
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, 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 >
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);
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> --- drivers/gpu/drm/panel/panel-dsi-cm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)