Re: [PATCH v1 2/2] drm/panel-simple: Add missing BUS descriptions for some panels
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
>> Signed-off-by: Dmitry Osipenko
>> ---
>> 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
Thanks to you and Sam!
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/panel-simple: Add missing BUS descriptions for some panels
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 > > -- > Regards, > > Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/panel-simple: Add missing BUS descriptions for some panels
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
> Signed-off-by: Dmitry Osipenko
> ---
> 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
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/panel-simple: Add missing BUS descriptions for some panels
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
> > >> Signed-off-by: Dmitry Osipenko
> > >> ---
> > >> 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
> >
> > Thanks to you and Sam!
>
> --
> Regards,
>
> Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/panel-simple: Add missing BUS descriptions for some panels
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
> >> Signed-off-by: Dmitry Osipenko
> >> ---
> >> 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
>
> Thanks to you and Sam!
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 2/2] drm/panel-simple: Add missing BUS descriptions for some panels
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
> Signed-off-by: Dmitry Osipenko
> ---
> 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
> .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,
> };
>
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
