| Message ID | 20200621222742.25695-3-digetx@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | Improve descriptions of a few simple-panels | expand |
Hi Dmitry, Thank you for the patch. On Mon, Jun 22, 2020 at 01:27:42AM +0300, Dmitry Osipenko wrote: > This patch adds missing BUS fields to the display panel descriptions of > the panels which are found on NVIDIA Tegra devices: > > 1. AUO B101AW03 > 2. Chunghwa CLAA070WP03XG > 3. Chunghwa CLAA101WA01A > 4. Chunghwa CLAA101WB01 > 5. Innolux N156BGE L21 > 6. Samsung LTN101NT05 > > Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/gpu/drm/panel/panel-simple.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index 87edd2bdf09a..986df9937650 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -698,6 +698,8 @@ static const struct panel_desc auo_b101aw03 = { > .width = 223, > .height = 125, > }, > + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, Does DRM_BUS_FLAG_PIXDATA_DRIVE_* make sense for LVDS ? The rest looks good, except the Samsung panel for which I haven't been able to locate a datasheet. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > @@ -1352,6 +1354,8 @@ static const struct panel_desc chunghwa_claa070wp03xg = { > .width = 94, > .height = 150, > }, > + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > @@ -1375,6 +1379,8 @@ static const struct panel_desc chunghwa_claa101wa01a = { > .width = 220, > .height = 120, > }, > + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > @@ -1398,6 +1404,8 @@ static const struct panel_desc chunghwa_claa101wb01 = { > .width = 223, > .height = 125, > }, > + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > @@ -2071,6 +2079,8 @@ static const struct panel_desc innolux_n156bge_l21 = { > .width = 344, > .height = 193, > }, > + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; > > @@ -3018,6 +3028,8 @@ static const struct panel_desc samsung_ltn101nt05 = { > .width = 223, > .height = 125, > }, > + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > .connector_type = DRM_MODE_CONNECTOR_LVDS, > }; >
27.06.2020 23:43, Laurent Pinchart пишет: > Hi Dmitry, > > Thank you for the patch. > > On Mon, Jun 22, 2020 at 01:27:42AM +0300, Dmitry Osipenko wrote: >> This patch adds missing BUS fields to the display panel descriptions of >> the panels which are found on NVIDIA Tegra devices: >> >> 1. AUO B101AW03 >> 2. Chunghwa CLAA070WP03XG >> 3. Chunghwa CLAA101WA01A >> 4. Chunghwa CLAA101WB01 >> 5. Innolux N156BGE L21 >> 6. Samsung LTN101NT05 >> >> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/gpu/drm/panel/panel-simple.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c >> index 87edd2bdf09a..986df9937650 100644 >> --- a/drivers/gpu/drm/panel/panel-simple.c >> +++ b/drivers/gpu/drm/panel/panel-simple.c >> @@ -698,6 +698,8 @@ static const struct panel_desc auo_b101aw03 = { >> .width = 223, >> .height = 125, >> }, >> + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, >> + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > > Does DRM_BUS_FLAG_PIXDATA_DRIVE_* make sense for LVDS ? To be honest I don't know whether it make sense or not for LVDS. I saw that other LVDS panels in panel-simple.c use the PIXDATA flag and then looked at what timing diagrams in the datasheets show. > The rest looks good, except the Samsung panel for which I haven't been > able to locate a datasheet. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks to you and Sam!
Hi Dmitry, On Sun, Jun 28, 2020 at 02:44:15AM +0300, Dmitry Osipenko wrote: > 27.06.2020 23:43, Laurent Pinchart пишет: > > On Mon, Jun 22, 2020 at 01:27:42AM +0300, Dmitry Osipenko wrote: > >> This patch adds missing BUS fields to the display panel descriptions of > >> the panels which are found on NVIDIA Tegra devices: > >> > >> 1. AUO B101AW03 > >> 2. Chunghwa CLAA070WP03XG > >> 3. Chunghwa CLAA101WA01A > >> 4. Chunghwa CLAA101WB01 > >> 5. Innolux N156BGE L21 > >> 6. Samsung LTN101NT05 > >> > >> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >> --- > >> drivers/gpu/drm/panel/panel-simple.c | 12 ++++++++++++ > >> 1 file changed, 12 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > >> index 87edd2bdf09a..986df9937650 100644 > >> --- a/drivers/gpu/drm/panel/panel-simple.c > >> +++ b/drivers/gpu/drm/panel/panel-simple.c > >> @@ -698,6 +698,8 @@ static const struct panel_desc auo_b101aw03 = { > >> .width = 223, > >> .height = 125, > >> }, > >> + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > >> + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > > > > Does DRM_BUS_FLAG_PIXDATA_DRIVE_* make sense for LVDS ? > > To be honest I don't know whether it make sense or not for LVDS. I saw > that other LVDS panels in panel-simple.c use the PIXDATA flag and then > looked at what timing diagrams in the datasheets show. I think we should drop DRM_BUS_FLAG_PIXDATA_DRIVE_* for LVDS panels. I'll submit a patch. > > The rest looks good, except the Samsung panel for which I haven't been > > able to locate a datasheet. > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thanks to you and Sam!
Hi Laurent. On Sun, Jun 28, 2020 at 10:07:45AM +0300, Laurent Pinchart wrote: > Hi Dmitry, > > On Sun, Jun 28, 2020 at 02:44:15AM +0300, Dmitry Osipenko wrote: > > 27.06.2020 23:43, Laurent Pinchart пишет: > > > On Mon, Jun 22, 2020 at 01:27:42AM +0300, Dmitry Osipenko wrote: > > >> This patch adds missing BUS fields to the display panel descriptions of > > >> the panels which are found on NVIDIA Tegra devices: > > >> > > >> 1. AUO B101AW03 > > >> 2. Chunghwa CLAA070WP03XG > > >> 3. Chunghwa CLAA101WA01A > > >> 4. Chunghwa CLAA101WB01 > > >> 5. Innolux N156BGE L21 > > >> 6. Samsung LTN101NT05 > > >> > > >> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > > >> --- > > >> drivers/gpu/drm/panel/panel-simple.c | 12 ++++++++++++ > > >> 1 file changed, 12 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > > >> index 87edd2bdf09a..986df9937650 100644 > > >> --- a/drivers/gpu/drm/panel/panel-simple.c > > >> +++ b/drivers/gpu/drm/panel/panel-simple.c > > >> @@ -698,6 +698,8 @@ static const struct panel_desc auo_b101aw03 = { > > >> .width = 223, > > >> .height = 125, > > >> }, > > >> + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > > >> + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > > > > > > Does DRM_BUS_FLAG_PIXDATA_DRIVE_* make sense for LVDS ? > > > > To be honest I don't know whether it make sense or not for LVDS. I saw > > that other LVDS panels in panel-simple.c use the PIXDATA flag and then > > looked at what timing diagrams in the datasheets show. > > I think we should drop DRM_BUS_FLAG_PIXDATA_DRIVE_* for LVDS panels. > I'll submit a patch. We should also clean up all the DRM_BUS_FLAG_* one day. No need for the deprecated values, so a few files needs an update. And we could document what flags makes sense for LVDS etc. On the TODO list... Sam > > > > The rest looks good, except the Samsung panel for which I haven't been > > > able to locate a datasheet. > > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Thanks to you and Sam! > > -- > Regards, > > Laurent Pinchart
Hi Sam, On Sun, Jun 28, 2020 at 09:52:45AM +0200, Sam Ravnborg wrote: > On Sun, Jun 28, 2020 at 10:07:45AM +0300, Laurent Pinchart wrote: > > On Sun, Jun 28, 2020 at 02:44:15AM +0300, Dmitry Osipenko wrote: > >> 27.06.2020 23:43, Laurent Pinchart пишет: > >>> On Mon, Jun 22, 2020 at 01:27:42AM +0300, Dmitry Osipenko wrote: > >>>> This patch adds missing BUS fields to the display panel descriptions of > >>>> the panels which are found on NVIDIA Tegra devices: > >>>> > >>>> 1. AUO B101AW03 > >>>> 2. Chunghwa CLAA070WP03XG > >>>> 3. Chunghwa CLAA101WA01A > >>>> 4. Chunghwa CLAA101WB01 > >>>> 5. Innolux N156BGE L21 > >>>> 6. Samsung LTN101NT05 > >>>> > >>>> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >>>> --- > >>>> drivers/gpu/drm/panel/panel-simple.c | 12 ++++++++++++ > >>>> 1 file changed, 12 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > >>>> index 87edd2bdf09a..986df9937650 100644 > >>>> --- a/drivers/gpu/drm/panel/panel-simple.c > >>>> +++ b/drivers/gpu/drm/panel/panel-simple.c > >>>> @@ -698,6 +698,8 @@ static const struct panel_desc auo_b101aw03 = { > >>>> .width = 223, > >>>> .height = 125, > >>>> }, > >>>> + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, > >>>> + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > >>> > >>> Does DRM_BUS_FLAG_PIXDATA_DRIVE_* make sense for LVDS ? > >> > >> To be honest I don't know whether it make sense or not for LVDS. I saw > >> that other LVDS panels in panel-simple.c use the PIXDATA flag and then > >> looked at what timing diagrams in the datasheets show. > > > > I think we should drop DRM_BUS_FLAG_PIXDATA_DRIVE_* for LVDS panels. > > I'll submit a patch. > > We should also clean up all the DRM_BUS_FLAG_* one day. > No need for the deprecated values, so a few files needs an update. > And we could document what flags makes sense for LVDS etc. Where would you add that documentation ? The hardest part is to find a place that will be noticed by developers :-) I've just submitted a patch that adds a WARN_ON to catch similar issues in the panel-simple driver. It's not ideal as we really shouldn't have such code in the kernel, this is something that should be caught as part of the integration process. > On the TODO list... > > >>> The rest looks good, except the Samsung panel for which I haven't been > >>> able to locate a datasheet. > >>> > >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
On Sun, Jun 28, 2020 at 11:02:53AM +0300, Laurent Pinchart wrote: > Hi Sam, > > > We should also clean up all the DRM_BUS_FLAG_* one day. > > No need for the deprecated values, so a few files needs an update. > > And we could document what flags makes sense for LVDS etc. > > Where would you add that documentation ? The hardest part is to find a > place that will be noticed by developers :-) I will try to extend drm_bus_flags documentation in drm_connector.h And then add a few comments in panel-simple as well. Sam > > I've just submitted a patch that adds a WARN_ON to catch similar issues > in the panel-simple driver. It's not ideal as we really shouldn't have > such code in the kernel, this is something that should be caught as part > of the integration process. > > > On the TODO list... > > > > >>> The rest looks good, except the Samsung panel for which I haven't been > > >>> able to locate a datasheet. > > >>> > > >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > -- > Regards, > > Laurent Pinchart
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 87edd2bdf09a..986df9937650 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -698,6 +698,8 @@ static const struct panel_desc auo_b101aw03 = { .width = 223, .height = 125, }, + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, .connector_type = DRM_MODE_CONNECTOR_LVDS, }; @@ -1352,6 +1354,8 @@ static const struct panel_desc chunghwa_claa070wp03xg = { .width = 94, .height = 150, }, + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, .connector_type = DRM_MODE_CONNECTOR_LVDS, }; @@ -1375,6 +1379,8 @@ static const struct panel_desc chunghwa_claa101wa01a = { .width = 220, .height = 120, }, + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, .connector_type = DRM_MODE_CONNECTOR_LVDS, }; @@ -1398,6 +1404,8 @@ static const struct panel_desc chunghwa_claa101wb01 = { .width = 223, .height = 125, }, + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, .connector_type = DRM_MODE_CONNECTOR_LVDS, }; @@ -2071,6 +2079,8 @@ static const struct panel_desc innolux_n156bge_l21 = { .width = 344, .height = 193, }, + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, .connector_type = DRM_MODE_CONNECTOR_LVDS, }; @@ -3018,6 +3028,8 @@ static const struct panel_desc samsung_ltn101nt05 = { .width = 223, .height = 125, }, + .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, .connector_type = DRM_MODE_CONNECTOR_LVDS, };
This patch adds missing BUS fields to the display panel descriptions of the panels which are found on NVIDIA Tegra devices: 1. AUO B101AW03 2. Chunghwa CLAA070WP03XG 3. Chunghwa CLAA101WA01A 4. Chunghwa CLAA101WB01 5. Innolux N156BGE L21 6. Samsung LTN101NT05 Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/gpu/drm/panel/panel-simple.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)