When I run:
scripts/kernel-doc -rst drivers/gpu/drm/panel/panel-simple.c
I see that several of the kernel-doc entries aren't showing up because
they don't specify the full path down the hierarchy. Let's fix that
and also move to inline kernel docs.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v4:
- ("drm: panel: simple: Fixup the struct panel_desc kernel doc") new for v4.
drivers/gpu/drm/panel/panel-simple.c | 59 ++++++++++++++++++++--------
1 file changed, 42 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 597f676a6591..813c90274631 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -64,33 +64,58 @@ struct panel_desc {
unsigned int bpc;
- /**
- * @width: width (in millimeters) of the panel's active display area
- * @height: height (in millimeters) of the panel's active display area
- */
struct {
+ /**
+ * @size.width: Width (in mm) of the active display area.
+ */
unsigned int width;
+
+ /**
+ * @size.height: Height (in mm) of the active display area.
+ */
unsigned int height;
} size;
- /**
- * @prepare: the time (in milliseconds) that it takes for the panel to
- * become ready and start receiving video data
- * @hpd_absent_delay: Add this to the prepare delay if we know Hot
- * Plug Detect isn't used.
- * @enable: the time (in milliseconds) that it takes for the panel to
- * display the first valid frame after starting to receive
- * video data
- * @disable: the time (in milliseconds) that it takes for the panel to
- * turn the display off (no content is visible)
- * @unprepare: the time (in milliseconds) that it takes for the panel
- * to power itself down completely
- */
struct {
+ /**
+ * @delay.prepare: Time for the panel to become ready.
+ *
+ * The time (in milliseconds) that it takes for the panel to
+ * become ready and start receiving video data
+ */
unsigned int prepare;
+
+ /**
+ * @delay.hpd_absent_delay: Time to wait if HPD isn't hooked up.
+ *
+ * Add this to the prepare delay if we know Hot Plug Detect
+ * isn't used.
+ */
unsigned int hpd_absent_delay;
+
+ /**
+ * @delay.enable: Time for the panel to display a valid frame.
+ *
+ * The time (in milliseconds) that it takes for the panel to
+ * display the first valid frame after starting to receive
+ * video data.
+ */
unsigned int enable;
+
+ /**
+ * @delay.disable: Time for the panel to turn the display off.
+ *
+ * The time (in milliseconds) that it takes for the panel to
+ * turn the display off (no content is visible).
+ */
unsigned int disable;
+
+ /**
+ * @delay.unprepare: Time to power down completely.
+ *
+ * The time (in milliseconds) that it takes for the panel
+ * to power itself down completely.
+ */
unsigned int unprepare;
} delay;
--
2.29.2.222.g5d2a92d10f8-goog
Add support for the BOE NV110WTM-N61 panel. The EDID lists two modes
(one for 60 Hz refresh rate and one for 40 Hz), so we'll list both of
them here.
Note that the panel datasheet requires 80 ms between HPD asserting and
the backlight power being turned on. We'll use the new timing
constraints structure to do this cleanly. This assumes that the
backlight will be enabled _after_ the panel enable finishes. This is
how it works today and seems a sane assumption.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v4:
- Rebased atop changes in earlier patches.
Changes in v2:
- Adjust for shorter names in patch #1.
drivers/gpu/drm/panel/panel-simple.c | 46 ++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index a54f42cb3adc..9bb723696e94 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1396,6 +1396,49 @@ static const struct panel_desc boe_nv101wxmn51 = {
},
};
+static const struct drm_display_mode boe_nv110wtm_n61_modes[] = {
+ {
+ .clock = 207800,
+ .hdisplay = 2160,
+ .hsync_start = 2160 + 48,
+ .hsync_end = 2160 + 48 + 32,
+ .htotal = 2160 + 48 + 32 + 100,
+ .vdisplay = 1440,
+ .vsync_start = 1440 + 3,
+ .vsync_end = 1440 + 3 + 6,
+ .vtotal = 1440 + 3 + 6 + 31,
+ },
+ {
+ .clock = 138500,
+ .hdisplay = 2160,
+ .hsync_start = 2160 + 48,
+ .hsync_end = 2160 + 48 + 32,
+ .htotal = 2160 + 48 + 32 + 100,
+ .vdisplay = 1440,
+ .vsync_start = 1440 + 3,
+ .vsync_end = 1440 + 3 + 6,
+ .vtotal = 1440 + 3 + 6 + 31,
+ },
+};
+
+static const struct panel_desc boe_nv110wtm_n61 = {
+ .modes = boe_nv110wtm_n61_modes,
+ .num_modes = ARRAY_SIZE(boe_nv110wtm_n61_modes),
+ .bpc = 8,
+ .size = {
+ .width = 233,
+ .height = 155,
+ },
+ .delay = {
+ .hpd_absent_delay = 200,
+ .prepare_to_enable = 80,
+ .unprepare = 500,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+ .bus_flags = DRM_BUS_FLAG_DATA_MSB_TO_LSB,
+ .connector_type = DRM_MODE_CONNECTOR_eDP,
+};
+
/* Also used for boe_nv133fhm_n62 */
static const struct drm_display_mode boe_nv133fhm_n61_modes = {
.clock = 147840,
@@ -4086,6 +4129,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "boe,nv101wxmn51",
.data = &boe_nv101wxmn51,
+ }, {
+ .compatible = "boe,nv110wtm-n61",
+ .data = &boe_nv110wtm_n61,
}, {
.compatible = "boe,nv133fhm-n61",
.data = &boe_nv133fhm_n61,
--
2.29.2.222.g5d2a92d10f8-goog
It is believed that all of the current users of the "unprepare" delay
don't actually need to wait the amount of time specified directly in
the unprepare phase. The purpose of the delay that's specified is to
allow the panel to fully power off so that we don't try to power it
back on before it's managed to full power down.
Let's use this observation to avoid the fixed delay that we currently
have. Instead of delaying, we'll note the current time when the
unprepare happens. If someone then tries to prepare the panel later
and not enough time has passed, we'll do the delay before starting the
prepare phase.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v4:
- Split ("Allow timing constraints, not fixed delays") into 2 patches.
- Reuse the existing "unprepare" and assume it means "unprepare_to_prepare".
- Shortened function name to panel_simple_wait().
- Change to fix kerneldoc is now patch #1 in the series.
drivers/gpu/drm/panel/panel-simple.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 813c90274631..4bc61d71f068 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -115,6 +115,11 @@ struct panel_desc {
*
* The time (in milliseconds) that it takes for the panel
* to power itself down completely.
+ *
+ * This time is used to prevent a future "prepare" from
+ * starting until at least this many milliseconds has passed.
+ * If at prepare time less time has passed since unprepare
+ * finished, the driver waits for the remaining time.
*/
unsigned int unprepare;
} delay;
@@ -130,6 +135,8 @@ struct panel_simple {
bool enabled;
bool no_hpd;
+ ktime_t unprepared_time;
+
const struct panel_desc *desc;
struct regulator *supply;
@@ -257,6 +264,20 @@ static int panel_simple_get_non_edid_modes(struct panel_simple *panel,
return num;
}
+static void panel_simple_wait(ktime_t start_ktime, unsigned int min_ms)
+{
+ ktime_t now_ktime, min_ktime;
+
+ if (!min_ms)
+ return;
+
+ min_ktime = ktime_add(start_ktime, ms_to_ktime(min_ms));
+ now_ktime = ktime_get();
+
+ if (ktime_before(now_ktime, min_ktime))
+ msleep(ktime_to_ms(ktime_sub(min_ktime, now_ktime)) + 1);
+}
+
static int panel_simple_disable(struct drm_panel *panel)
{
struct panel_simple *p = to_panel_simple(panel);
@@ -283,10 +304,8 @@ static int panel_simple_unprepare(struct drm_panel *panel)
regulator_disable(p->supply);
- if (p->desc->delay.unprepare)
- msleep(p->desc->delay.unprepare);
-
p->prepared = false;
+ p->unprepared_time = ktime_get();
return 0;
}
@@ -326,6 +345,8 @@ static int panel_simple_prepare(struct drm_panel *panel)
if (p->prepared)
return 0;
+ panel_simple_wait(p->unprepared_time, p->desc->delay.unprepare);
+
err = regulator_enable(p->supply);
if (err < 0) {
dev_err(panel->dev, "failed to enable supply: %d\n", err);
--
2.29.2.222.g5d2a92d10f8-goog
Add yet another eDP panel.
Signed-off-by: Douglas Anderson <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
(no changes since v1)
.../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
index f9750b0b6708..994c19c357de 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -76,6 +76,8 @@ properties:
# BOE OPTOELECTRONICS TECHNOLOGY 10.1" WXGA TFT LCD panel
- boe,nv101wxmn51
# BOE NV133FHM-N61 13.3" FHD (1920x1080) TFT LCD Panel
+ - boe,nv110wtm-n61
+ # BOE NV110WTM-N61 11.0" 2160x1440 TFT LCD Panel
- boe,nv133fhm-n61
# BOE NV133FHM-N62 13.3" FHD (1920x1080) TFT LCD Panel
- boe,nv133fhm-n62
--
2.29.2.222.g5d2a92d10f8-goog
On the panel I'm looking at, there's an 80 ms minimum time between HPD
being asserted by the panel and setting the backlight enable GPIO.
While we could just add an 80 ms "enable" delay, this is not ideal.
Link training is allowed to happen in parallel with this delay so the
fixed 80 ms delay over-delays.
We'll support this by logging the time at the end of prepare and then
delaying in enable if enough time hasn't passed.
Signed-off-by: Douglas Anderson <[email protected]>
---
Changes in v4:
- Split ("Allow timing constraints, not fixed delays") into 2 patches.
- Fixed kerneldoc.
drivers/gpu/drm/panel/panel-simple.c | 44 ++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 4bc61d71f068..a54f42cb3adc 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -93,6 +93,36 @@ struct panel_desc {
*/
unsigned int hpd_absent_delay;
+ /**
+ * @delay.prepare_to_enable: Time between prepare and enable.
+ *
+ * The minimum time, in milliseconds, that needs to have passed
+ * between when prepare finished and enable may begin. If at
+ * enable time less time has passed since prepare finished,
+ * the driver waits for the remaining time.
+ *
+ * If a fixed enable delay is also specified, we'll start
+ * counting before delaying for the fixed delay.
+ *
+ * If a fixed prepare delay is also specified, we won't start
+ * counting until after the fixed delay. We can't overlap this
+ * fixed delay with the min time because the fixed delay
+ * doesn't happen at the end of the function if a HPD GPIO was
+ * specified.
+ *
+ * In other words:
+ * prepare()
+ * ...
+ * // do fixed prepare delay
+ * // wait for HPD GPIO if applicable
+ * // start counting for prepare_to_enable
+ *
+ * enable()
+ * // do fixed enable delay
+ * // enforce prepare_to_enable min time
+ */
+ unsigned int prepare_to_enable;
+
/**
* @delay.enable: Time for the panel to display a valid frame.
*
@@ -131,10 +161,10 @@ struct panel_desc {
struct panel_simple {
struct drm_panel base;
- bool prepared;
bool enabled;
bool no_hpd;
+ ktime_t prepared_time;
ktime_t unprepared_time;
const struct panel_desc *desc;
@@ -297,14 +327,14 @@ static int panel_simple_unprepare(struct drm_panel *panel)
{
struct panel_simple *p = to_panel_simple(panel);
- if (!p->prepared)
+ if (p->prepared_time == 0)
return 0;
gpiod_set_value_cansleep(p->enable_gpio, 0);
regulator_disable(p->supply);
- p->prepared = false;
+ p->prepared_time = 0;
p->unprepared_time = ktime_get();
return 0;
@@ -342,7 +372,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
int err;
int hpd_asserted;
- if (p->prepared)
+ if (p->prepared_time != 0)
return 0;
panel_simple_wait(p->unprepared_time, p->desc->delay.unprepare);
@@ -381,7 +411,7 @@ static int panel_simple_prepare(struct drm_panel *panel)
}
}
- p->prepared = true;
+ p->prepared_time = ktime_get();
return 0;
}
@@ -396,6 +426,8 @@ static int panel_simple_enable(struct drm_panel *panel)
if (p->desc->delay.enable)
msleep(p->desc->delay.enable);
+ panel_simple_wait(p->prepared_time, p->desc->delay.prepare_to_enable);
+
p->enabled = true;
return 0;
@@ -562,7 +594,7 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
return -ENOMEM;
panel->enabled = false;
- panel->prepared = false;
+ panel->prepared_time = 0;
panel->desc = desc;
panel->no_hpd = of_property_read_bool(dev->of_node, "no-hpd");
--
2.29.2.222.g5d2a92d10f8-goog
Hi,
On Mon, Nov 9, 2020 at 5:01 PM Douglas Anderson <[email protected]> wrote:
>
> Add support for the BOE NV110WTM-N61 panel. The EDID lists two modes
> (one for 60 Hz refresh rate and one for 40 Hz), so we'll list both of
> them here.
>
> Note that the panel datasheet requires 80 ms between HPD asserting and
> the backlight power being turned on. We'll use the new timing
> constraints structure to do this cleanly. This assumes that the
> backlight will be enabled _after_ the panel enable finishes. This is
> how it works today and seems a sane assumption.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
> Changes in v4:
> - Rebased atop changes in earlier patches.
>
> Changes in v2:
> - Adjust for shorter names in patch #1.
>
> drivers/gpu/drm/panel/panel-simple.c | 46 ++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index a54f42cb3adc..9bb723696e94 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1396,6 +1396,49 @@ static const struct panel_desc boe_nv101wxmn51 = {
> },
> };
>
> +static const struct drm_display_mode boe_nv110wtm_n61_modes[] = {
> + {
> + .clock = 207800,
> + .hdisplay = 2160,
> + .hsync_start = 2160 + 48,
> + .hsync_end = 2160 + 48 + 32,
> + .htotal = 2160 + 48 + 32 + 100,
> + .vdisplay = 1440,
> + .vsync_start = 1440 + 3,
> + .vsync_end = 1440 + 3 + 6,
> + .vtotal = 1440 + 3 + 6 + 31,
> + },
> + {
> + .clock = 138500,
> + .hdisplay = 2160,
> + .hsync_start = 2160 + 48,
> + .hsync_end = 2160 + 48 + 32,
> + .htotal = 2160 + 48 + 32 + 100,
> + .vdisplay = 1440,
> + .vsync_start = 1440 + 3,
> + .vsync_end = 1440 + 3 + 6,
> + .vtotal = 1440 + 3 + 6 + 31,
> + },
I can do it as a follow-up patch if there is no other reason to spin,
but I realized that I forgot to add:
.flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
...to both the 40 and 60 Hz modes to make it match what's in the EDID.
I'm also OK w/ it getting added by a maintainer when this lands if
that makes sense.
-Doug
Hi,
On Mon, Nov 9, 2020 at 5:01 PM Douglas Anderson <[email protected]> wrote:
>
> When I run:
> scripts/kernel-doc -rst drivers/gpu/drm/panel/panel-simple.c
>
> I see that several of the kernel-doc entries aren't showing up because
> they don't specify the full path down the hierarchy. Let's fix that
> and also move to inline kernel docs.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
> Changes in v4:
> - ("drm: panel: simple: Fixup the struct panel_desc kernel doc") new for v4.
>
> drivers/gpu/drm/panel/panel-simple.c | 59 ++++++++++++++++++++--------
> 1 file changed, 42 insertions(+), 17 deletions(-)
I'm curious if there are any outstanding actions I need to take for
this series. I know I found that on patch #4 I should have added the
sync flags [1] but I was waiting to see if there was any other
feedback before sending a v5. If there's nothing else I'm happy with
any of:
a) I can send a v5 with that small fix.
b) v4 can land an the maintainer landing can add the ".flags =
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC," into the patch when
landing.
c) v4 can land and I can send a follow-up patch with the sync flags fix.
I know it's the holidays right now and folks are busy, but I just
wanted to see if there were any actions I needed to take. Personally
I'm on vacation the last 3 days this week and I imagine I'll come back
to work Monday swamped with email, so today/tomorrow would be an ideal
time to send a spin if one is needed. Thanks!
-Doug
[1] https://lore.kernel.org/r/CAD=FV=UPkuJ5E2sCQeozNR3CO+LZW=DW9h77vfooeiSMqGnYpA@mail.gmail.com
Hi Douglas,
On Mon, Nov 09, 2020 at 05:00:55PM -0800, Douglas Anderson wrote:
> When I run:
> scripts/kernel-doc -rst drivers/gpu/drm/panel/panel-simple.c
>
> I see that several of the kernel-doc entries aren't showing up because
> they don't specify the full path down the hierarchy. Let's fix that
> and also move to inline kernel docs.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Thanks, applied to drm-misc-next.
Could you do a follow-up patch that moves the rest as inline comments
and verify that all fields are described. (W=1 should show no warnings).
That would be appreciated!
Sam
Hi Douglas,
On Mon, Nov 09, 2020 at 05:00:56PM -0800, Douglas Anderson wrote:
> It is believed that all of the current users of the "unprepare" delay
> don't actually need to wait the amount of time specified directly in
> the unprepare phase. The purpose of the delay that's specified is to
> allow the panel to fully power off so that we don't try to power it
> back on before it's managed to full power down.
>
> Let's use this observation to avoid the fixed delay that we currently
> have. Instead of delaying, we'll note the current time when the
> unprepare happens. If someone then tries to prepare the panel later
> and not enough time has passed, we'll do the delay before starting the
> prepare phase.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Applied to drm-misc-next.
Sam
Hi Douglas,
On Mon, Nov 09, 2020 at 05:00:57PM -0800, Douglas Anderson wrote:
> On the panel I'm looking at, there's an 80 ms minimum time between HPD
> being asserted by the panel and setting the backlight enable GPIO.
> While we could just add an 80 ms "enable" delay, this is not ideal.
> Link training is allowed to happen in parallel with this delay so the
> fixed 80 ms delay over-delays.
>
> We'll support this by logging the time at the end of prepare and then
> delaying in enable if enough time hasn't passed.
>
> Signed-off-by: Douglas Anderson <[email protected]>
Applied too.
Sam
Hi Douglas,
On Mon, Nov 09, 2020 at 05:00:58PM -0800, Douglas Anderson wrote:
> Add support for the BOE NV110WTM-N61 panel. The EDID lists two modes
> (one for 60 Hz refresh rate and one for 40 Hz), so we'll list both of
> them here.
>
> Note that the panel datasheet requires 80 ms between HPD asserting and
> the backlight power being turned on. We'll use the new timing
> constraints structure to do this cleanly. This assumes that the
> backlight will be enabled _after_ the panel enable finishes. This is
> how it works today and seems a sane assumption.
>
> Signed-off-by: Douglas Anderson <[email protected]>
I applied the binding patch (bindings before driver patch),
and added the missing flags while applying this patch.
All to drm-misc-next.
Sam
Hi,
On Sun, Nov 29, 2020 at 2:10 PM Sam Ravnborg <[email protected]> wrote:
>
> Hi Douglas,
> On Mon, Nov 09, 2020 at 05:00:55PM -0800, Douglas Anderson wrote:
> > When I run:
> > scripts/kernel-doc -rst drivers/gpu/drm/panel/panel-simple.c
> >
> > I see that several of the kernel-doc entries aren't showing up because
> > they don't specify the full path down the hierarchy. Let's fix that
> > and also move to inline kernel docs.
> >
> > Signed-off-by: Douglas Anderson <[email protected]>
>
> Thanks, applied to drm-misc-next.
>
> Could you do a follow-up patch that moves the rest as inline comments
> and verify that all fields are described. (W=1 should show no warnings).
> That would be appreciated!
Ah, I hadn't realized that you wanted it all moved and thought
originally you just wanted the deeply nested stuff. See if:
https://lore.kernel.org/r/20201201125822.1.I3c4191336014bd57364309439e56f600c94bb12b@changeid
...looks like what you want. :-)
-Doug