diff mbox series

[v2,3/3] drm/panel: panel-edp: Fix AUO 0x405c panel naming and add a variant

Message ID 20240228011133.1238439-4-hsinyi@chromium.org
State New
Headers
Series Match panel with id and name |

Commit Message

Hsin-Yi Wang Feb. 28, 2024, 1:06 a.m. UTC 
  There are 2 different AUO panels using the same panel id. One of the
variants requires using overridden modes to resolve glitching issue as
described in commit 70e0d5550f5c ("drm/panel-edp: Add auo_b116xa3_mode").
Other variants should use the modes parsed from EDID.

Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
v2: new
---
 drivers/gpu/drm/panel/panel-edp.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Doug Anderson Feb. 29, 2024, 12:22 a.m. UTC | #1 
Hi,

On Tue, Feb 27, 2024 at 5:11 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> There are 2 different AUO panels using the same panel id. One of the
> variants requires using overridden modes to resolve glitching issue as
> described in commit 70e0d5550f5c ("drm/panel-edp: Add auo_b116xa3_mode").
> Other variants should use the modes parsed from EDID.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> v2: new
> ---
>  drivers/gpu/drm/panel/panel-edp.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)

The previous version of this patch that we reverted also had an
override for AUO 0x615c. Is that one no longer needed?


> @@ -1990,7 +2003,9 @@ static const struct edp_panel_entry edp_panels[] = {
>         EDP_PANEL_ENTRY('A', 'U', 'O', 0x239b, &delay_200_500_e50, "B116XAN06.1"),
>         EDP_PANEL_ENTRY('A', 'U', 'O', 0x255c, &delay_200_500_e50, "B116XTN02.5"),
>         EDP_PANEL_ENTRY('A', 'U', 'O', 0x403d, &delay_200_500_e50, "B140HAN04.0"),
> -       EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0"),
> +       EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAN04.0 "),
> +       EDP_PANEL_ENTRY2('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0 ",
> +                        &auo_b116xa3_mode),

The name string now has a space at the end of it. I _guess_ that's OK.
Hmmm, but I guess you should update the kernel doc for "struct
edp_panel_entry". The name field is described as "Name of this panel
(for printing to logs)". Now it should include that it's also used for
matching EDIDs in some cases too.
Hsin-Yi Wang Feb. 29, 2024, 1:04 a.m. UTC | #2 
On Wed, Feb 28, 2024 at 4:22 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Feb 27, 2024 at 5:11 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
> >
> > There are 2 different AUO panels using the same panel id. One of the
> > variants requires using overridden modes to resolve glitching issue as
> > described in commit 70e0d5550f5c ("drm/panel-edp: Add auo_b116xa3_mode").
> > Other variants should use the modes parsed from EDID.
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > ---
> > v2: new
> > ---
> >  drivers/gpu/drm/panel/panel-edp.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
>
> The previous version of this patch that we reverted also had an
> override for AUO 0x615c. Is that one no longer needed?
>
>
> > @@ -1990,7 +2003,9 @@ static const struct edp_panel_entry edp_panels[] = {
> >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x239b, &delay_200_500_e50, "B116XAN06.1"),
> >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x255c, &delay_200_500_e50, "B116XTN02.5"),
> >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x403d, &delay_200_500_e50, "B140HAN04.0"),
> > -       EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0"),
> > +       EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAN04.0 "),
> > +       EDP_PANEL_ENTRY2('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0 ",
> > +                        &auo_b116xa3_mode),
>
> The name string now has a space at the end of it. I _guess_ that's OK.
> Hmmm, but I guess you should update the kernel doc for "struct
> edp_panel_entry". The name field is described as "Name of this panel
> (for printing to logs)". Now it should include that it's also used for
> matching EDIDs in some cases too.

The space here is because in the EDID, there is space at the end,
before 0x0a (\n).
Okay I will update the kernel doc to mention that the same should be
exactly the same as the panel name.
Dmitry Baryshkov Feb. 29, 2024, 1:12 a.m. UTC | #3 
On Thu, 29 Feb 2024 at 03:05, Hsin-Yi Wang <hsinyi@chromium.org> wrote:
>
> On Wed, Feb 28, 2024 at 4:22 PM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Tue, Feb 27, 2024 at 5:11 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote:
> > >
> > > There are 2 different AUO panels using the same panel id. One of the
> > > variants requires using overridden modes to resolve glitching issue as
> > > described in commit 70e0d5550f5c ("drm/panel-edp: Add auo_b116xa3_mode").
> > > Other variants should use the modes parsed from EDID.
> > >
> > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > > ---
> > > v2: new
> > > ---
> > >  drivers/gpu/drm/panel/panel-edp.c | 17 ++++++++++++++++-
> > >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > The previous version of this patch that we reverted also had an
> > override for AUO 0x615c. Is that one no longer needed?
> >
> >
> > > @@ -1990,7 +2003,9 @@ static const struct edp_panel_entry edp_panels[] = {
> > >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x239b, &delay_200_500_e50, "B116XAN06.1"),
> > >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x255c, &delay_200_500_e50, "B116XTN02.5"),
> > >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x403d, &delay_200_500_e50, "B140HAN04.0"),
> > > -       EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0"),
> > > +       EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAN04.0 "),
> > > +       EDP_PANEL_ENTRY2('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0 ",
> > > +                        &auo_b116xa3_mode),
> >
> > The name string now has a space at the end of it. I _guess_ that's OK.
> > Hmmm, but I guess you should update the kernel doc for "struct
> > edp_panel_entry". The name field is described as "Name of this panel
> > (for printing to logs)". Now it should include that it's also used for
> > matching EDIDs in some cases too.
>
> The space here is because in the EDID, there is space at the end,
> before 0x0a (\n).
> Okay I will update the kernel doc to mention that the same should be
> exactly the same as the panel name.

Maybe it would be better to strip all the whitespace on the right?
Hsin-Yi Wang Feb. 29, 2024, 1:17 a.m. UTC | #4 
On Wed, Feb 28, 2024 at 5:13 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Thu, 29 Feb 2024 at 03:05, Hsin-Yi Wang <hsinyi@chromium.org> wrote:
> >
> > On Wed, Feb 28, 2024 at 4:22 PM Doug Anderson <dianders@chromium.org> wrote:
> > >
> > > Hi,
> > >
> > > On Tue, Feb 27, 2024 at 5:11 PM Hsin-Yi Wang <hsinyi@chromiumorg> wrote:
> > > >
> > > > There are 2 different AUO panels using the same panel id. One of the
> > > > variants requires using overridden modes to resolve glitching issue as
> > > > described in commit 70e0d5550f5c ("drm/panel-edp: Add auo_b116xa3_mode").
> > > > Other variants should use the modes parsed from EDID.
> > > >
> > > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > > > ---
> > > > v2: new
> > > > ---
> > > >  drivers/gpu/drm/panel/panel-edp.c | 17 ++++++++++++++++-
> > > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > >
> > > The previous version of this patch that we reverted also had an
> > > override for AUO 0x615c. Is that one no longer needed?
> > >
> > >
> > > > @@ -1990,7 +2003,9 @@ static const struct edp_panel_entry edp_panels[] = {
> > > >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x239b, &delay_200_500_e50, "B116XAN06.1"),
> > > >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x255c, &delay_200_500_e50, "B116XTN02.5"),
> > > >         EDP_PANEL_ENTRY('A', 'U', 'O', 0x403d, &delay_200_500_e50, "B140HAN04.0"),
> > > > -       EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0"),
> > > > +       EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAN04.0 "),
> > > > +       EDP_PANEL_ENTRY2('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0 ",
> > > > +                        &auo_b116xa3_mode),
> > >
> > > The name string now has a space at the end of it. I _guess_ that's OK.
> > > Hmmm, but I guess you should update the kernel doc for "struct
> > > edp_panel_entry". The name field is described as "Name of this panel
> > > (for printing to logs)". Now it should include that it's also used for
> > > matching EDIDs in some cases too.
> >
> > The space here is because in the EDID, there is space at the end,
> > before 0x0a (\n).
> > Okay I will update the kernel doc to mention that the same should be
> > exactly the same as the panel name.
>
> Maybe it would be better to strip all the whitespace on the right?
>

Sounds good too.

> --
> With best wishes
> Dmitry
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index 72ad552bff24..e39af92342e8 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1013,6 +1013,19 @@  static const struct panel_desc auo_b101ean01 = {
 	},
 };
 
+static const struct drm_display_mode auo_b116xa3_mode = {
+	.clock = 70589,
+	.hdisplay = 1366,
+	.hsync_start = 1366 + 40,
+	.hsync_end = 1366 + 40 + 40,
+	.htotal = 1366 + 40 + 40 + 32,
+	.vdisplay = 768,
+	.vsync_start = 768 + 10,
+	.vsync_end = 768 + 10 + 12,
+	.vtotal = 768 + 10 + 12 + 6,
+	.flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
 static const struct drm_display_mode auo_b116xak01_mode = {
 	.clock = 69300,
 	.hdisplay = 1366,
@@ -1990,7 +2003,9 @@  static const struct edp_panel_entry edp_panels[] = {
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x239b, &delay_200_500_e50, "B116XAN06.1"),
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x255c, &delay_200_500_e50, "B116XTN02.5"),
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x403d, &delay_200_500_e50, "B140HAN04.0"),
-	EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0"),
+	EDP_PANEL_ENTRY('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAN04.0 "),
+	EDP_PANEL_ENTRY2('A', 'U', 'O', 0x405c, &auo_b116xak01.delay, "B116XAK01.0 ",
+			 &auo_b116xa3_mode),
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x435c, &delay_200_500_e50, "Unknown"),
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x582d, &delay_200_500_e50, "B133UAN01.0"),
 	EDP_PANEL_ENTRY('A', 'U', 'O', 0x615c, &delay_200_500_e50, "B116XAN06.1"),