Re: [PATCH] drm/panel-edp: Add timings for BOE NV133WUM-N63

2023-11-02 Thread Clayton Craft 
On Mon, 30 Oct 2023 13:58:31 -0700 Jessica Zhang  
wrote:
> 
> 
> On 10/27/2023 7:19 PM, Clayton Craft wrote:
> > This panel is found on laptops e.g., variants of the Thinkpad X13s.
> > Configuration was collected from the panel's EDID.
> 
> Hi Clayton,
> 
> Thanks for the patch -- it looks good to me aside from one minor comment 
> below.
> 
> > 
> > Signed-off-by: Clayton Craft 
> > ---
> >   drivers/gpu/drm/panel/panel-edp.c | 27 +++
> >   1 file changed, 27 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c 
> > b/drivers/gpu/drm/panel/panel-edp.c
> > index 95c8472d878a..5db283f014f3 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -1204,6 +1204,29 @@ static const struct panel_desc boe_nv133fhm_n61 = {
> > },
> >   };
> >   
> > +static const struct drm_display_mode boe_nv133wum_n63_modes = {
> 
> Will other modes be added to this struct in the future? If so, I think 
> we can probably turn this into an array to make it easier to extend.

To be honest, I have no idea. I saw a kernel backtrace in dmesg that was related
to this driver not having config for my panel, so I added it after looking at
similar examples here.

Personally I'd prefer not to get too fancy with arrays and such until there's
actually a need for it, e.g., someone comes along with more modes to add. But I
can try to do that if my "keep it simple until it needs to be extended" approach
is unacceptable :)

> Otherwise, can you change the name to "*_mode"? Seems to me that almost 
> all other panels with a single mode have the name as "*_mode" with the 
> only exception being a carry-over from older panel-simple.c code.

Ok I'll rename to *_mode in V2. Thanks for reviewing!

-Clayton


signature.asc
Description: PGP signature

Re: [PATCH] drm/panel-edp: Add timings for BOE NV133WUM-N63

2023-10-30 Thread Jessica Zhang 




On 10/27/2023 7:19 PM, Clayton Craft wrote:

This panel is found on laptops e.g., variants of the Thinkpad X13s.
Configuration was collected from the panel's EDID.


Hi Clayton,

Thanks for the patch -- it looks good to me aside from one minor comment 
below.




Signed-off-by: Clayton Craft 
---
  drivers/gpu/drm/panel/panel-edp.c | 27 +++
  1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-edp.c 
b/drivers/gpu/drm/panel/panel-edp.c
index 95c8472d878a..5db283f014f3 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1204,6 +1204,29 @@ static const struct panel_desc boe_nv133fhm_n61 = {
},
  };
  
+static const struct drm_display_mode boe_nv133wum_n63_modes = {


Will other modes be added to this struct in the future? If so, I think 
we can probably turn this into an array to make it easier to extend.


Otherwise, can you change the name to "*_mode"? Seems to me that almost 
all other panels with a single mode have the name as "*_mode" with the 
only exception being a carry-over from older panel-simple.c code.


Thanks,

Jessica Zhang


+   .clock = 157760,
+   .hdisplay = 1920,
+   .hsync_start = 1920 + 48,
+   .hsync_end = 1920 + 48 + 32,
+   .htotal = 1920 + 48 + 32 + 80,
+   .vdisplay = 1200,
+   .vsync_start = 1200 + 3,
+   .vsync_end = 1200 + 3 + 6,
+   .vtotal = 1200 + 3 + 6 + 31,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc boe_nv133wum_n63 = {
+   .modes = _nv133wum_n63_modes,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 286,
+   .height = 179,
+   },
+};
+
  static const struct drm_display_mode boe_nv140fhmn49_modes[] = {
{
.clock = 148500,
@@ -1723,6 +1746,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "boe,nv133fhm-n62",
.data = _nv133fhm_n61,
+   }, {
+   .compatible = "boe,nv133wum-n63",
+   .data = _nv133wum_n63,
}, {
.compatible = "boe,nv140fhmn49",
.data = _nv140fhmn49,
@@ -1852,6 +1878,7 @@ static const struct edp_panel_entry edp_panels[] = {
EDP_PANEL_ENTRY('B', 'O', 'E', 0x095f, _200_500_e50, "NE135FBM-N41 
v8.1"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x098d, _nv110wtm_n61.delay, 
"NV110WTM-N61"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x09dd, _200_500_e50, 
"NT116WHM-N21"),
+   EDP_PANEL_ENTRY('B', 'O', 'E', 0x0a1b, _200_500_e50, 
"NV133WUM-N63"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x0a5d, _200_500_e50, 
"NV116WHM-N45"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x0ac5, _200_500_e50, 
"NV116WHM-N4C"),
  
--

2.40.1

[PATCH] drm/panel-edp: Add timings for BOE NV133WUM-N63

2023-10-28 Thread Clayton Craft 
This panel is found on laptops e.g., variants of the Thinkpad X13s.
Configuration was collected from the panel's EDID.

Signed-off-by: Clayton Craft 
---
 drivers/gpu/drm/panel/panel-edp.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-edp.c 
b/drivers/gpu/drm/panel/panel-edp.c
index 95c8472d878a..5db283f014f3 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1204,6 +1204,29 @@ static const struct panel_desc boe_nv133fhm_n61 = {
},
 };
 
+static const struct drm_display_mode boe_nv133wum_n63_modes = {
+   .clock = 157760,
+   .hdisplay = 1920,
+   .hsync_start = 1920 + 48,
+   .hsync_end = 1920 + 48 + 32,
+   .htotal = 1920 + 48 + 32 + 80,
+   .vdisplay = 1200,
+   .vsync_start = 1200 + 3,
+   .vsync_end = 1200 + 3 + 6,
+   .vtotal = 1200 + 3 + 6 + 31,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc boe_nv133wum_n63 = {
+   .modes = _nv133wum_n63_modes,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 286,
+   .height = 179,
+   },
+};
+
 static const struct drm_display_mode boe_nv140fhmn49_modes[] = {
{
.clock = 148500,
@@ -1723,6 +1746,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "boe,nv133fhm-n62",
.data = _nv133fhm_n61,
+   }, {
+   .compatible = "boe,nv133wum-n63",
+   .data = _nv133wum_n63,
}, {
.compatible = "boe,nv140fhmn49",
.data = _nv140fhmn49,
@@ -1852,6 +1878,7 @@ static const struct edp_panel_entry edp_panels[] = {
EDP_PANEL_ENTRY('B', 'O', 'E', 0x095f, _200_500_e50, 
"NE135FBM-N41 v8.1"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x098d, _nv110wtm_n61.delay, 
"NV110WTM-N61"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x09dd, _200_500_e50, 
"NT116WHM-N21"),
+   EDP_PANEL_ENTRY('B', 'O', 'E', 0x0a1b, _200_500_e50, 
"NV133WUM-N63"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x0a5d, _200_500_e50, 
"NV116WHM-N45"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x0ac5, _200_500_e50, 
"NV116WHM-N4C"),
 
-- 
2.40.1

[PATCH] drm/panel-edp: Add timings for BOE NV133WUM-N63

2023-10-27 Thread Clayton Craft 
This panel is found on laptops e.g., variants of the Thinkpad X13s.
Configuration was collected from the panel's EDID.

Signed-off-by: Clayton Craft 
---
 drivers/gpu/drm/panel/panel-edp.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-edp.c 
b/drivers/gpu/drm/panel/panel-edp.c
index 95c8472d878a..5db283f014f3 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1204,6 +1204,29 @@ static const struct panel_desc boe_nv133fhm_n61 = {
},
 };
 
+static const struct drm_display_mode boe_nv133wum_n63_modes = {
+   .clock = 157760,
+   .hdisplay = 1920,
+   .hsync_start = 1920 + 48,
+   .hsync_end = 1920 + 48 + 32,
+   .htotal = 1920 + 48 + 32 + 80,
+   .vdisplay = 1200,
+   .vsync_start = 1200 + 3,
+   .vsync_end = 1200 + 3 + 6,
+   .vtotal = 1200 + 3 + 6 + 31,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc boe_nv133wum_n63 = {
+   .modes = _nv133wum_n63_modes,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 286,
+   .height = 179,
+   },
+};
+
 static const struct drm_display_mode boe_nv140fhmn49_modes[] = {
{
.clock = 148500,
@@ -1723,6 +1746,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "boe,nv133fhm-n62",
.data = _nv133fhm_n61,
+   }, {
+   .compatible = "boe,nv133wum-n63",
+   .data = _nv133wum_n63,
}, {
.compatible = "boe,nv140fhmn49",
.data = _nv140fhmn49,
@@ -1852,6 +1878,7 @@ static const struct edp_panel_entry edp_panels[] = {
EDP_PANEL_ENTRY('B', 'O', 'E', 0x095f, _200_500_e50, 
"NE135FBM-N41 v8.1"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x098d, _nv110wtm_n61.delay, 
"NV110WTM-N61"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x09dd, _200_500_e50, 
"NT116WHM-N21"),
+   EDP_PANEL_ENTRY('B', 'O', 'E', 0x0a1b, _200_500_e50, 
"NV133WUM-N63"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x0a5d, _200_500_e50, 
"NV116WHM-N45"),
EDP_PANEL_ENTRY('B', 'O', 'E', 0x0ac5, _200_500_e50, 
"NV116WHM-N4C"),
 
-- 
2.40.1