* [v5 1/5] drm/panel: add basic DP AUX backlight support
2021-05-30 15:56 [v5 0/5] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20 Rajeev Nandan
@ 2021-05-30 15:56 ` Rajeev Nandan
2021-05-30 18:49 ` kernel test robot
` (2 more replies)
2021-05-30 15:56 ` [v5 2/5] drm/panel-simple: Support DP AUX backlight Rajeev Nandan
` (3 subsequent siblings)
4 siblings, 3 replies; 14+ messages in thread
From: Rajeev Nandan @ 2021-05-30 15:56 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: daniel.thompson, Rajeev Nandan, mkrishn, jani.nikula,
linux-kernel, abhinavk, dianders, a.hajda, thierry.reding,
seanpaul, laurent.pinchart, kalyan_t, hoegsberg, sam
Some panels support backlight control over DP AUX channel using
VESA's standard backlight control interface.
Using new DRM eDP backlight helpers, add support to create and
register a backlight for those panels in drm_panel to simplify
the panel drivers.
The panel driver with access to "struct drm_dp_aux" can create and
register a backlight device using following code snippet in its
probe() function:
err = drm_panel_dp_aux_backlight(panel, aux);
if (err)
return err;
Then drm_panel will handle backlight_(enable|disable) calls
similar to the case when drm_panel_of_backlight() is used.
Currently, we are not supporting one feature where the source
device can combine the backlight brightness levels set through
DP AUX and the BL_PWM_DIM eDP connector pin. Since it's not
required for the basic backlight controls, it can be added later.
Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
---
Changes in v5:
- New (Douglas) [1]
[1] https://lore.kernel.org/dri-devel/CAD=FV=UuWuKibpT15McweuZ24qxsSAsSvJ3Q2MbZqgw5oggBVQ@mail.gmail.com/
drivers/gpu/drm/drm_panel.c | 108 ++++++++++++++++++++++++++++++++++++++++++++
include/drm/drm_panel.h | 15 ++++--
2 files changed, 119 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index f634371..f290315 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -26,12 +26,20 @@
#include <linux/module.h>
#include <drm/drm_crtc.h>
+#include <drm/drm_dp_helper.h>
#include <drm/drm_panel.h>
#include <drm/drm_print.h>
static DEFINE_MUTEX(panel_lock);
static LIST_HEAD(panel_list);
+struct dp_aux_backlight {
+ struct backlight_device *base;
+ struct drm_dp_aux *aux;
+ struct drm_edp_backlight_info info;
+ bool enabled;
+};
+
/**
* DOC: drm panel
*
@@ -342,6 +350,106 @@ int drm_panel_of_backlight(struct drm_panel *panel)
return 0;
}
EXPORT_SYMBOL(drm_panel_of_backlight);
+
+static int dp_aux_backlight_update_status(struct backlight_device *bd)
+{
+ struct dp_aux_backlight *bl = bl_get_data(bd);
+ u16 brightness = backlight_get_brightness(bd);
+ int ret = 0;
+
+ if (brightness > 0) {
+ if (!bl->enabled) {
+ drm_edp_backlight_enable(bl->aux, &bl->info, brightness);
+ bl->enabled = true;
+ return 0;
+ }
+ ret = drm_edp_backlight_set_level(bl->aux, &bl->info, brightness);
+ } else {
+ if (bl->enabled) {
+ drm_edp_backlight_disable(bl->aux, &bl->info);
+ bl->enabled = false;
+ }
+ }
+
+ return ret;
+}
+
+static const struct backlight_ops dp_aux_bl_ops = {
+ .update_status = dp_aux_backlight_update_status,
+};
+
+/**
+ * drm_panel_dp_aux_backlight - create and use DP AUX backlight
+ * @panel: DRM panel
+ * @aux: The DP AUX channel to use
+ *
+ * Use this function to create and handle backlight if your panel
+ * supports backlight control over DP AUX channel using DPCD
+ * registers as per VESA's standard backlight control interface.
+ *
+ * When the panel is enabled backlight will be enabled after a
+ * successful call to &drm_panel_funcs.enable()
+ *
+ * When the panel is disabled backlight will be disabled before the
+ * call to &drm_panel_funcs.disable().
+ *
+ * A typical implementation for a panel driver supporting backlight
+ * control over DP AUX will call this function at probe time.
+ * Backlight will then be handled transparently without requiring
+ * any intervention from the driver.
+ *
+ * drm_panel_dp_aux_backlight() must be called after the call to drm_panel_init().
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
+{
+ struct dp_aux_backlight *bl;
+ struct backlight_properties props = { 0 };
+ u16 current_level;
+ u8 current_mode;
+ u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
+ int ret;
+
+ if (!panel || !panel->dev || !aux)
+ return -EINVAL;
+
+ bl = devm_kzalloc(panel->dev, sizeof(*bl), GFP_KERNEL);
+ if (!bl)
+ return -ENOMEM;
+
+ bl->aux = aux;
+
+ ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd,
+ EDP_DISPLAY_CTL_CAP_SIZE);
+ if (ret < 0)
+ return ret;
+
+ if (!drm_edp_backlight_supported(edp_dpcd)) {
+ DRM_DEV_INFO(panel->dev, "DP AUX backlight is not supported\n");
+ return 0;
+ }
+
+ ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
+ ¤t_level, ¤t_mode);
+ if (ret < 0)
+ return ret;
+
+ props.type = BACKLIGHT_RAW;
+ props.brightness = current_level;
+ props.max_brightness = bl->info.max;
+
+ bl->base = devm_backlight_device_register(panel->dev, "dp_aux_backlight",
+ panel->dev, bl,
+ &dp_aux_bl_ops, &props);
+ if (IS_ERR(bl->base))
+ return PTR_ERR(bl->base);
+
+ panel->backlight = bl->base;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_panel_dp_aux_backlight);
#endif
MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 33605c3..b1ba4de 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -32,6 +32,7 @@ struct backlight_device;
struct device_node;
struct drm_connector;
struct drm_device;
+struct drm_dp_aux;
struct drm_panel;
struct display_timing;
@@ -64,8 +65,8 @@ enum drm_panel_orientation;
* the panel. This is the job of the .unprepare() function.
*
* Backlight can be handled automatically if configured using
- * drm_panel_of_backlight(). Then the driver does not need to implement the
- * functionality to enable/disable backlight.
+ * drm_panel_of_backlight() or drm_panel_dp_aux_backlight(). Then the driver
+ * does not need to implement the functionality to enable/disable backlight.
*/
struct drm_panel_funcs {
/**
@@ -144,8 +145,8 @@ struct drm_panel {
* Backlight device, used to turn on backlight after the call
* to enable(), and to turn off backlight before the call to
* disable().
- * backlight is set by drm_panel_of_backlight() and drivers
- * shall not assign it.
+ * backlight is set by drm_panel_of_backlight()/drm_panel_dp_aux_backlight()
+ * and drivers shall not assign it.
*/
struct backlight_device *backlight;
@@ -208,11 +209,17 @@ static inline int of_drm_get_panel_orientation(const struct device_node *np,
#if IS_ENABLED(CONFIG_DRM_PANEL) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
(IS_MODULE(CONFIG_DRM) && IS_MODULE(CONFIG_BACKLIGHT_CLASS_DEVICE)))
int drm_panel_of_backlight(struct drm_panel *panel);
+int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux);
#else
static inline int drm_panel_of_backlight(struct drm_panel *panel)
{
return 0;
}
+static inline int drm_panel_dp_aux_backlight(struct drm_panel *panel,
+ struct drm_dp_aux *aux)
+{
+ return 0;
+}
#endif
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [v5 1/5] drm/panel: add basic DP AUX backlight support
2021-05-30 15:56 ` [v5 1/5] drm/panel: add basic DP AUX backlight support Rajeev Nandan
@ 2021-05-30 18:49 ` kernel test robot
2021-05-30 18:52 ` kernel test robot
2021-06-03 0:05 ` Doug Anderson
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-05-30 18:49 UTC (permalink / raw)
To: Rajeev Nandan, dri-devel, linux-arm-msm, freedreno, devicetree
Cc: kbuild-all, Rajeev Nandan, linux-kernel, dianders, thierry.reding, sam
[-- Attachment #1: Type: text/plain, Size: 4142 bytes --]
Hi Rajeev,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.13-rc3 next-20210528]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Rajeev-Nandan/drm-Support-basic-DPCD-backlight-in-panel-simple-and-add-a-new-panel-ATNA33XC20/20210530-235810
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: openrisc-randconfig-r002-20210530 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/1cf1b3f6b1f766d4e932422d1cdec19354acdcc3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rajeev-Nandan/drm-Support-basic-DPCD-backlight-in-panel-simple-and-add-a-new-panel-ATNA33XC20/20210530-235810
git checkout 1cf1b3f6b1f766d4e932422d1cdec19354acdcc3
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=openrisc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/drm_panel.c:39:32: error: field 'info' has incomplete type
39 | struct drm_edp_backlight_info info;
| ^~~~
drivers/gpu/drm/drm_panel.c: In function 'dp_aux_backlight_update_status':
>> drivers/gpu/drm/drm_panel.c:362:4: error: implicit declaration of function 'drm_edp_backlight_enable'; did you mean 'backlight_enable'? [-Werror=implicit-function-declaration]
362 | drm_edp_backlight_enable(bl->aux, &bl->info, brightness);
| ^~~~~~~~~~~~~~~~~~~~~~~~
| backlight_enable
>> drivers/gpu/drm/drm_panel.c:366:9: error: implicit declaration of function 'drm_edp_backlight_set_level' [-Werror=implicit-function-declaration]
366 | ret = drm_edp_backlight_set_level(bl->aux, &bl->info, brightness);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/drm_panel.c:369:4: error: implicit declaration of function 'drm_edp_backlight_disable'; did you mean 'backlight_disable'? [-Werror=implicit-function-declaration]
369 | drm_edp_backlight_disable(bl->aux, &bl->info);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| backlight_disable
drivers/gpu/drm/drm_panel.c: In function 'drm_panel_dp_aux_backlight':
>> drivers/gpu/drm/drm_panel.c:428:7: error: implicit declaration of function 'drm_edp_backlight_supported' [-Werror=implicit-function-declaration]
428 | if (!drm_edp_backlight_supported(edp_dpcd)) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/drm_panel.c:433:8: error: implicit declaration of function 'drm_edp_backlight_init' [-Werror=implicit-function-declaration]
433 | ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
| ^~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for LOCKDEP
Depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
Selected by
- PROVE_LOCKING && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
- DEBUG_LOCK_ALLOC && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
vim +/info +39 drivers/gpu/drm/drm_panel.c
35
36 struct dp_aux_backlight {
37 struct backlight_device *base;
38 struct drm_dp_aux *aux;
> 39 struct drm_edp_backlight_info info;
40 bool enabled;
41 };
42
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35681 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v5 1/5] drm/panel: add basic DP AUX backlight support
2021-05-30 15:56 ` [v5 1/5] drm/panel: add basic DP AUX backlight support Rajeev Nandan
2021-05-30 18:49 ` kernel test robot
@ 2021-05-30 18:52 ` kernel test robot
2021-06-03 0:05 ` Doug Anderson
2 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-05-30 18:52 UTC (permalink / raw)
To: Rajeev Nandan, dri-devel, linux-arm-msm, freedreno, devicetree
Cc: kbuild-all, Rajeev Nandan, linux-kernel, dianders,
clang-built-linux, thierry.reding, sam
[-- Attachment #1: Type: text/plain, Size: 4261 bytes --]
Hi Rajeev,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.13-rc3 next-20210528]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Rajeev-Nandan/drm-Support-basic-DPCD-backlight-in-panel-simple-and-add-a-new-panel-ATNA33XC20/20210530-235810
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: arm64-randconfig-r005-20210530 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project bc6799f2f79f0ae87e9f1ebf9d25ba799fbd25a9)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/1cf1b3f6b1f766d4e932422d1cdec19354acdcc3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rajeev-Nandan/drm-Support-basic-DPCD-backlight-in-panel-simple-and-add-a-new-panel-ATNA33XC20/20210530-235810
git checkout 1cf1b3f6b1f766d4e932422d1cdec19354acdcc3
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/drm_panel.c:39:32: error: field has incomplete type 'struct drm_edp_backlight_info'
struct drm_edp_backlight_info info;
^
drivers/gpu/drm/drm_panel.c:39:9: note: forward declaration of 'struct drm_edp_backlight_info'
struct drm_edp_backlight_info info;
^
>> drivers/gpu/drm/drm_panel.c:362:4: error: implicit declaration of function 'drm_edp_backlight_enable' [-Werror,-Wimplicit-function-declaration]
drm_edp_backlight_enable(bl->aux, &bl->info, brightness);
^
drivers/gpu/drm/drm_panel.c:362:4: note: did you mean 'backlight_enable'?
include/linux/backlight.h:363:19: note: 'backlight_enable' declared here
static inline int backlight_enable(struct backlight_device *bd)
^
>> drivers/gpu/drm/drm_panel.c:366:9: error: implicit declaration of function 'drm_edp_backlight_set_level' [-Werror,-Wimplicit-function-declaration]
ret = drm_edp_backlight_set_level(bl->aux, &bl->info, brightness);
^
>> drivers/gpu/drm/drm_panel.c:369:4: error: implicit declaration of function 'drm_edp_backlight_disable' [-Werror,-Wimplicit-function-declaration]
drm_edp_backlight_disable(bl->aux, &bl->info);
^
drivers/gpu/drm/drm_panel.c:369:4: note: did you mean 'backlight_disable'?
include/linux/backlight.h:379:19: note: 'backlight_disable' declared here
static inline int backlight_disable(struct backlight_device *bd)
^
>> drivers/gpu/drm/drm_panel.c:428:7: error: implicit declaration of function 'drm_edp_backlight_supported' [-Werror,-Wimplicit-function-declaration]
if (!drm_edp_backlight_supported(edp_dpcd)) {
^
>> drivers/gpu/drm/drm_panel.c:433:8: error: implicit declaration of function 'drm_edp_backlight_init' [-Werror,-Wimplicit-function-declaration]
ret = drm_edp_backlight_init(aux, &bl->info, 0, edp_dpcd,
^
6 errors generated.
vim +39 drivers/gpu/drm/drm_panel.c
35
36 struct dp_aux_backlight {
37 struct backlight_device *base;
38 struct drm_dp_aux *aux;
> 39 struct drm_edp_backlight_info info;
40 bool enabled;
41 };
42
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 42938 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v5 1/5] drm/panel: add basic DP AUX backlight support
2021-05-30 15:56 ` [v5 1/5] drm/panel: add basic DP AUX backlight support Rajeev Nandan
2021-05-30 18:49 ` kernel test robot
2021-05-30 18:52 ` kernel test robot
@ 2021-06-03 0:05 ` Doug Anderson
2021-06-03 6:26 ` rajeevny
2 siblings, 1 reply; 14+ messages in thread
From: Doug Anderson @ 2021-06-03 0:05 UTC (permalink / raw)
To: Rajeev Nandan
Cc: linux-fbdev, dri-devel, Andrzej Hajda, Thierry Reding,
Laurent Pinchart, Kristian H. Kristensen, Sam Ravnborg,
Daniel Thompson, Lee Jones,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jani Nikula, linux-arm-msm, Abhinav Kumar, Sean Paul,
Kalyan Thota, Krishna Manikandan, Jingoo Han, LKML, freedreno
Hi,
On Sun, May 30, 2021 at 8:57 AM Rajeev Nandan <rajeevny@codeaurora.org> wrote:
>
> +static int dp_aux_backlight_update_status(struct backlight_device *bd)
> +{
> + struct dp_aux_backlight *bl = bl_get_data(bd);
> + u16 brightness = backlight_get_brightness(bd);
> + int ret = 0;
> +
> + if (brightness > 0) {
> + if (!bl->enabled) {
> + drm_edp_backlight_enable(bl->aux, &bl->info, brightness);
> + bl->enabled = true;
> + return 0;
> + }
> + ret = drm_edp_backlight_set_level(bl->aux, &bl->info, brightness);
> + } else {
> + if (bl->enabled) {
> + drm_edp_backlight_disable(bl->aux, &bl->info);
> + bl->enabled = false;
> + }
> + }
I was trying to figure out if there are any races / locking problems /
problems trying to tweak the backlight when the panel is off. I don't
_think_ there are. Specifically:
1. Before turning the panel off, drm_panel will call
backlight_disable(). That will set BL_CORE_FBBLANK which is not set by
any other calls. Then it will call your
dp_aux_backlight_update_status().
2. Once BL_CORE_FBBLANK is set then backlight_get_brightness() will
always return 0.
This means that a transition from 0 -> non-zero (and enable) will
always only happen when the panel is on, which is good. It also means
that we'll always transition to 0 (disable the backlight) when the
panel turns off.
In terms of other races, it looks like the AUX transfer code handles
grabbing a mutex around transfers so we should be safe there.
So I guess the above is just a long-winded way of saying that this
looks right to me. :-)
BTW: we should probably make sure that the full set of people
identified by `./scripts/get_maintainer.pl -f
./drivers/video/backlight` are CCed on your series. I see Daniel
already and I've added the rest.
> +/**
> + * drm_panel_dp_aux_backlight - create and use DP AUX backlight
> + * @panel: DRM panel
> + * @aux: The DP AUX channel to use
> + *
> + * Use this function to create and handle backlight if your panel
> + * supports backlight control over DP AUX channel using DPCD
> + * registers as per VESA's standard backlight control interface.
> + *
> + * When the panel is enabled backlight will be enabled after a
> + * successful call to &drm_panel_funcs.enable()
> + *
> + * When the panel is disabled backlight will be disabled before the
> + * call to &drm_panel_funcs.disable().
> + *
> + * A typical implementation for a panel driver supporting backlight
> + * control over DP AUX will call this function at probe time.
> + * Backlight will then be handled transparently without requiring
> + * any intervention from the driver.
> + *
> + * drm_panel_dp_aux_backlight() must be called after the call to drm_panel_init().
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux)
> +{
> + struct dp_aux_backlight *bl;
> + struct backlight_properties props = { 0 };
> + u16 current_level;
> + u8 current_mode;
> + u8 edp_dpcd[EDP_DISPLAY_CTL_CAP_SIZE];
> + int ret;
> +
> + if (!panel || !panel->dev || !aux)
> + return -EINVAL;
> +
> + bl = devm_kzalloc(panel->dev, sizeof(*bl), GFP_KERNEL);
> + if (!bl)
> + return -ENOMEM;
> +
> + bl->aux = aux;
> +
> + ret = drm_dp_dpcd_read(aux, DP_EDP_DPCD_REV, edp_dpcd,
> + EDP_DISPLAY_CTL_CAP_SIZE);
> + if (ret < 0)
> + return ret;
> +
> + if (!drm_edp_backlight_supported(edp_dpcd)) {
> + DRM_DEV_INFO(panel->dev, "DP AUX backlight is not supported\n");
> + return 0;
> + }
nit: move this part up above the memory allocation. There's no reason
to allocate memory for "bl" if the backlight isn't supported.
> @@ -64,8 +65,8 @@ enum drm_panel_orientation;
> * the panel. This is the job of the .unprepare() function.
> *
> * Backlight can be handled automatically if configured using
> - * drm_panel_of_backlight(). Then the driver does not need to implement the
> - * functionality to enable/disable backlight.
> + * drm_panel_of_backlight() or drm_panel_dp_aux_backlight(). Then the driver
> + * does not need to implement the functionality to enable/disable backlight.
> */
> struct drm_panel_funcs {
> /**
> @@ -144,8 +145,8 @@ struct drm_panel {
> * Backlight device, used to turn on backlight after the call
> * to enable(), and to turn off backlight before the call to
> * disable().
> - * backlight is set by drm_panel_of_backlight() and drivers
> - * shall not assign it.
> + * backlight is set by drm_panel_of_backlight()/drm_panel_dp_aux_backlight()
> + * and drivers shall not assign it.
Slight nit that I would have wrapped the drm_panel_dp_aux_backlight()
to the next line just to avoid one really long line followed by a
short one.
Other than the two nits (ordering of memory allocation and word
wrapping in a comment), this looks good to me. Feel free to add my
Reviewed-by tag when you fix the nits.
NOTE: Even though I have commit access to drm-misc now, I wouldn't
feel comfortable merging this to drm-misc myself without review
feedback from someone more senior. Obviously we're still blocked on my
and Lyude's series landing first, but even assuming those just land
as-is we'll need some more adult supervision before this can land. ;-)
That being said, I personally think this looks pretty nice now.
-Doug
> */
> struct backlight_device *backlight;
>
> @@ -208,11 +209,17 @@ static inline int of_drm_get_panel_orientation(const struct device_node *np,
> #if IS_ENABLED(CONFIG_DRM_PANEL) && (IS_BUILTIN(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
> (IS_MODULE(CONFIG_DRM) && IS_MODULE(CONFIG_BACKLIGHT_CLASS_DEVICE)))
> int drm_panel_of_backlight(struct drm_panel *panel);
> +int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux);
> #else
> static inline int drm_panel_of_backlight(struct drm_panel *panel)
> {
> return 0;
> }
> +static inline int drm_panel_dp_aux_backlight(struct drm_panel *panel,
> + struct drm_dp_aux *aux)
> +{
> + return 0;
> +}
> #endif
>
> #endif
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v5 1/5] drm/panel: add basic DP AUX backlight support
2021-06-03 0:05 ` Doug Anderson
@ 2021-06-03 6:26 ` rajeevny
0 siblings, 0 replies; 14+ messages in thread
From: rajeevny @ 2021-06-03 6:26 UTC (permalink / raw)
To: Doug Anderson
Cc: linux-fbdev, dri-devel, Andrzej Hajda, Thierry Reding,
Laurent Pinchart, Kristian H. Kristensen, Sam Ravnborg,
Daniel Thompson, Lee Jones,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jani Nikula, linux-arm-msm, Abhinav Kumar, Sean Paul,
Kalyan Thota, Krishna Manikandan, Jingoo Han, LKML, freedreno
On 03-06-2021 05:35, Doug Anderson wrote:
> Hi,
>
> On Sun, May 30, 2021 at 8:57 AM Rajeev Nandan <rajeevny@codeaurora.org>
> wrote:
>>
>
> Other than the two nits (ordering of memory allocation and word
> wrapping in a comment), this looks good to me. Feel free to add my
> Reviewed-by tag when you fix the nits.
>
> NOTE: Even though I have commit access to drm-misc now, I wouldn't
> feel comfortable merging this to drm-misc myself without review
> feedback from someone more senior. Obviously we're still blocked on my
> and Lyude's series landing first, but even assuming those just land
> as-is we'll need some more adult supervision before this can land. ;-)
> That being said, I personally think this looks pretty nice now.
>
>
> -Doug
Thank you, Doug.
I'll address the review comments of this patch and another patch (v5
3/5)
in the next spin. I'll wait for Lyude to check this series, as she
wanted
to review it in a few days.
Thanks,
Rajeev
^ permalink raw reply [flat|nested] 14+ messages in thread
* [v5 2/5] drm/panel-simple: Support DP AUX backlight
2021-05-30 15:56 [v5 0/5] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20 Rajeev Nandan
2021-05-30 15:56 ` [v5 1/5] drm/panel: add basic DP AUX backlight support Rajeev Nandan
@ 2021-05-30 15:56 ` Rajeev Nandan
2021-05-30 19:25 ` kernel test robot
2021-06-03 0:05 ` Doug Anderson
2021-05-30 15:56 ` [v5 3/5] drm/panel-simple: Support for delays between GPIO & regulator Rajeev Nandan
` (2 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Rajeev Nandan @ 2021-05-30 15:56 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: daniel.thompson, Rajeev Nandan, mkrishn, jani.nikula,
linux-kernel, abhinavk, dianders, a.hajda, thierry.reding,
seanpaul, laurent.pinchart, kalyan_t, hoegsberg, sam
If there is no backlight specified in the device tree and the panel
has access to the DP AUX channel then create a DP AUX backlight if
supported by the panel.
Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
---
This patch depends on [1] (drm/panel: panel-simple: Stash DP AUX bus;
allow using it for DDC) and the previous patch (2/5) of this series.
Changes in v4:
- New
Changes in v5:
- Address review comments and move backlight functions to drm_panel.c (Douglas)
- Create and register DP AUX backlight if there is no backlight specified in the
device tree and panel has the DP AUX channel. (Douglas)
- The new drm_panel_dp_aux_backlight() will do the drm_edp_backlight_supported() check.
[1] https://lore.kernel.org/dri-devel/20210524165920.v8.7.I18e60221f6d048d14d6c50a770b15f356fa75092@changeid/
drivers/gpu/drm/panel/panel-simple.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index b09be6e..047fad5 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -800,6 +800,12 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,
if (err)
goto disable_pm_runtime;
+ if (!panel->base.backlight && panel->aux) {
+ err = drm_panel_dp_aux_backlight(&panel->base, panel->aux);
+ if (err)
+ goto disable_pm_runtime;
+ }
+
drm_panel_add(&panel->base);
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [v5 2/5] drm/panel-simple: Support DP AUX backlight
2021-05-30 15:56 ` [v5 2/5] drm/panel-simple: Support DP AUX backlight Rajeev Nandan
@ 2021-05-30 19:25 ` kernel test robot
2021-06-03 0:05 ` Doug Anderson
1 sibling, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-05-30 19:25 UTC (permalink / raw)
To: Rajeev Nandan, dri-devel, linux-arm-msm, freedreno, devicetree
Cc: kbuild-all, Rajeev Nandan, linux-kernel, dianders,
clang-built-linux, thierry.reding, sam
[-- Attachment #1: Type: text/plain, Size: 9957 bytes --]
Hi Rajeev,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.13-rc3 next-20210528]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Rajeev-Nandan/drm-Support-basic-DPCD-backlight-in-panel-simple-and-add-a-new-panel-ATNA33XC20/20210530-235810
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a006-20210530 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project bc6799f2f79f0ae87e9f1ebf9d25ba799fbd25a9)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/fedf88beabe2c179d593bbb61ff5df62ac909fa1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rajeev-Nandan/drm-Support-basic-DPCD-backlight-in-panel-simple-and-add-a-new-panel-ATNA33XC20/20210530-235810
git checkout fedf88beabe2c179d593bbb61ff5df62ac909fa1
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/panel/panel-simple.c:796:39: error: no member named 'aux' in 'struct panel_simple'
if (!panel->base.backlight && panel->aux) {
~~~~~ ^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:52: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
>> drivers/gpu/drm/panel/panel-simple.c:796:39: error: no member named 'aux' in 'struct panel_simple'
if (!panel->base.backlight && panel->aux) {
~~~~~ ^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:61: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
>> drivers/gpu/drm/panel/panel-simple.c:796:39: error: no member named 'aux' in 'struct panel_simple'
if (!panel->base.backlight && panel->aux) {
~~~~~ ^
include/linux/compiler.h:56:47: note: expanded from macro 'if'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~
include/linux/compiler.h:58:86: note: expanded from macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
include/linux/compiler.h:69:3: note: expanded from macro '__trace_if_value'
(cond) ? \
^~~~
drivers/gpu/drm/panel/panel-simple.c:797:57: error: no member named 'aux' in 'struct panel_simple'
err = drm_panel_dp_aux_backlight(&panel->base, panel->aux);
~~~~~ ^
4 errors generated.
vim +796 drivers/gpu/drm/panel/panel-simple.c
659
660 static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
661 {
662 struct panel_simple *panel;
663 struct display_timing dt;
664 struct device_node *ddc;
665 int connector_type;
666 u32 bus_flags;
667 int err;
668
669 panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
670 if (!panel)
671 return -ENOMEM;
672
673 panel->enabled = false;
674 panel->prepared_time = 0;
675 panel->desc = desc;
676
677 panel->no_hpd = of_property_read_bool(dev->of_node, "no-hpd");
678 if (!panel->no_hpd) {
679 err = panel_simple_get_hpd_gpio(dev, panel);
680 if (err)
681 return err;
682 }
683
684 panel->supply = devm_regulator_get(dev, "power");
685 if (IS_ERR(panel->supply))
686 return PTR_ERR(panel->supply);
687
688 panel->enable_gpio = devm_gpiod_get_optional(dev, "enable",
689 GPIOD_OUT_LOW);
690 if (IS_ERR(panel->enable_gpio)) {
691 err = PTR_ERR(panel->enable_gpio);
692 if (err != -EPROBE_DEFER)
693 dev_err(dev, "failed to request GPIO: %d\n", err);
694 return err;
695 }
696
697 err = of_drm_get_panel_orientation(dev->of_node, &panel->orientation);
698 if (err) {
699 dev_err(dev, "%pOF: failed to get orientation %d\n", dev->of_node, err);
700 return err;
701 }
702
703 ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0);
704 if (ddc) {
705 panel->ddc = of_find_i2c_adapter_by_node(ddc);
706 of_node_put(ddc);
707
708 if (!panel->ddc)
709 return -EPROBE_DEFER;
710 }
711
712 if (desc == &panel_dpi) {
713 /* Handle the generic panel-dpi binding */
714 err = panel_dpi_probe(dev, panel);
715 if (err)
716 goto free_ddc;
717 } else {
718 if (!of_get_display_timing(dev->of_node, "panel-timing", &dt))
719 panel_simple_parse_panel_timing_node(dev, panel, &dt);
720 }
721
722 connector_type = desc->connector_type;
723 /* Catch common mistakes for panels. */
724 switch (connector_type) {
725 case 0:
726 dev_warn(dev, "Specify missing connector_type\n");
727 connector_type = DRM_MODE_CONNECTOR_DPI;
728 break;
729 case DRM_MODE_CONNECTOR_LVDS:
730 WARN_ON(desc->bus_flags &
731 ~(DRM_BUS_FLAG_DE_LOW |
732 DRM_BUS_FLAG_DE_HIGH |
733 DRM_BUS_FLAG_DATA_MSB_TO_LSB |
734 DRM_BUS_FLAG_DATA_LSB_TO_MSB));
735 WARN_ON(desc->bus_format != MEDIA_BUS_FMT_RGB666_1X7X3_SPWG &&
736 desc->bus_format != MEDIA_BUS_FMT_RGB888_1X7X4_SPWG &&
737 desc->bus_format != MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA);
738 WARN_ON(desc->bus_format == MEDIA_BUS_FMT_RGB666_1X7X3_SPWG &&
739 desc->bpc != 6);
740 WARN_ON((desc->bus_format == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG ||
741 desc->bus_format == MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA) &&
742 desc->bpc != 8);
743 break;
744 case DRM_MODE_CONNECTOR_eDP:
745 if (desc->bus_format == 0)
746 dev_warn(dev, "Specify missing bus_format\n");
747 if (desc->bpc != 6 && desc->bpc != 8)
748 dev_warn(dev, "Expected bpc in {6,8} but got: %u\n", desc->bpc);
749 break;
750 case DRM_MODE_CONNECTOR_DSI:
751 if (desc->bpc != 6 && desc->bpc != 8)
752 dev_warn(dev, "Expected bpc in {6,8} but got: %u\n", desc->bpc);
753 break;
754 case DRM_MODE_CONNECTOR_DPI:
755 bus_flags = DRM_BUS_FLAG_DE_LOW |
756 DRM_BUS_FLAG_DE_HIGH |
757 DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE |
758 DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
759 DRM_BUS_FLAG_DATA_MSB_TO_LSB |
760 DRM_BUS_FLAG_DATA_LSB_TO_MSB |
761 DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE |
762 DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE;
763 if (desc->bus_flags & ~bus_flags)
764 dev_warn(dev, "Unexpected bus_flags(%d)\n", desc->bus_flags & ~bus_flags);
765 if (!(desc->bus_flags & bus_flags))
766 dev_warn(dev, "Specify missing bus_flags\n");
767 if (desc->bus_format == 0)
768 dev_warn(dev, "Specify missing bus_format\n");
769 if (desc->bpc != 6 && desc->bpc != 8)
770 dev_warn(dev, "Expected bpc in {6,8} but got: %u\n", desc->bpc);
771 break;
772 default:
773 dev_warn(dev, "Specify a valid connector_type: %d\n", desc->connector_type);
774 connector_type = DRM_MODE_CONNECTOR_DPI;
775 break;
776 }
777
778 dev_set_drvdata(dev, panel);
779
780 /*
781 * We use runtime PM for prepare / unprepare since those power the panel
782 * on and off and those can be very slow operations. This is important
783 * to optimize powering the panel on briefly to read the EDID before
784 * fully enabling the panel.
785 */
786 pm_runtime_enable(dev);
787 pm_runtime_set_autosuspend_delay(dev, 1000);
788 pm_runtime_use_autosuspend(dev);
789
790 drm_panel_init(&panel->base, dev, &panel_simple_funcs, connector_type);
791
792 err = drm_panel_of_backlight(&panel->base);
793 if (err)
794 goto disable_pm_runtime;
795
> 796 if (!panel->base.backlight && panel->aux) {
797 err = drm_panel_dp_aux_backlight(&panel->base, panel->aux);
798 if (err)
799 goto disable_pm_runtime;
800 }
801
802 drm_panel_add(&panel->base);
803
804 return 0;
805
806 disable_pm_runtime:
807 pm_runtime_disable(dev);
808 free_ddc:
809 if (panel->ddc)
810 put_device(&panel->ddc->dev);
811
812 return err;
813 }
814
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41588 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [v5 2/5] drm/panel-simple: Support DP AUX backlight
2021-05-30 15:56 ` [v5 2/5] drm/panel-simple: Support DP AUX backlight Rajeev Nandan
2021-05-30 19:25 ` kernel test robot
@ 2021-06-03 0:05 ` Doug Anderson
1 sibling, 0 replies; 14+ messages in thread
From: Doug Anderson @ 2021-06-03 0:05 UTC (permalink / raw)
To: Rajeev Nandan
Cc: linux-fbdev, dri-devel, Andrzej Hajda, Thierry Reding,
Laurent Pinchart, Kristian H. Kristensen, Sam Ravnborg,
Daniel Thompson, Lee Jones,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jani Nikula, linux-arm-msm, Abhinav Kumar, Sean Paul,
Kalyan Thota, Krishna Manikandan, Jingoo Han, LKML, freedreno
Hi,
On Sun, May 30, 2021 at 8:57 AM Rajeev Nandan <rajeevny@codeaurora.org> wrote:
>
> If there is no backlight specified in the device tree and the panel
> has access to the DP AUX channel then create a DP AUX backlight if
> supported by the panel.
>
> Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
> ---
>
> This patch depends on [1] (drm/panel: panel-simple: Stash DP AUX bus;
> allow using it for DDC) and the previous patch (2/5) of this series.
>
> Changes in v4:
> - New
>
> Changes in v5:
> - Address review comments and move backlight functions to drm_panel.c (Douglas)
> - Create and register DP AUX backlight if there is no backlight specified in the
> device tree and panel has the DP AUX channel. (Douglas)
> - The new drm_panel_dp_aux_backlight() will do the drm_edp_backlight_supported() check.
>
> [1] https://lore.kernel.org/dri-devel/20210524165920.v8.7.I18e60221f6d048d14d6c50a770b15f356fa75092@changeid/
>
> drivers/gpu/drm/panel/panel-simple.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index b09be6e..047fad5 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -800,6 +800,12 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,
> if (err)
> goto disable_pm_runtime;
>
> + if (!panel->base.backlight && panel->aux) {
> + err = drm_panel_dp_aux_backlight(&panel->base, panel->aux);
> + if (err)
> + goto disable_pm_runtime;
> + }
It's so nice now!
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [v5 3/5] drm/panel-simple: Support for delays between GPIO & regulator
2021-05-30 15:56 [v5 0/5] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20 Rajeev Nandan
2021-05-30 15:56 ` [v5 1/5] drm/panel: add basic DP AUX backlight support Rajeev Nandan
2021-05-30 15:56 ` [v5 2/5] drm/panel-simple: Support DP AUX backlight Rajeev Nandan
@ 2021-05-30 15:56 ` Rajeev Nandan
2021-06-03 0:05 ` Doug Anderson
2021-05-30 15:56 ` [v5 4/5] dt-bindings: display: simple: Add Samsung ATNA33XC20 Rajeev Nandan
2021-05-30 15:56 ` [v5 5/5] drm/panel-simple: " Rajeev Nandan
4 siblings, 1 reply; 14+ messages in thread
From: Rajeev Nandan @ 2021-05-30 15:56 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: daniel.thompson, Rajeev Nandan, mkrishn, jani.nikula,
linux-kernel, abhinavk, dianders, a.hajda, thierry.reding,
seanpaul, laurent.pinchart, kalyan_t, hoegsberg, sam
Some panels datasheets may specify a delay between the enable GPIO and
the regulator. Support this in panel-simple.
Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
---
Changes in v4:
- New
Changes in v5:
- Update description (Douglas)
- Warn if "power_to_enable" or "disable_to_power_off" is non-zero and panel->enable_gpio
is NULL (Douglas)
drivers/gpu/drm/panel/panel-simple.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index 047fad5..e3f5b7e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -133,6 +133,22 @@ struct panel_desc {
unsigned int prepare_to_enable;
/**
+ * @delay.power_to_enable: Time for the power to enable the display on.
+ *
+ * The time (in milliseconds) to wait after powering up the display
+ * before asserting its enable pin.
+ */
+ unsigned int power_to_enable;
+
+ /**
+ * @delay.disable_to_power_off: Time for the disable to power the display off.
+ *
+ * The time (in milliseconds) to wait before powering off the display
+ * after deasserting its enable pin.
+ */
+ unsigned int disable_to_power_off;
+
+ /**
* @delay.enable: Time for the panel to display a valid frame.
*
* The time (in milliseconds) that it takes for the panel to
@@ -347,6 +363,10 @@ static int panel_simple_suspend(struct device *dev)
struct panel_simple *p = dev_get_drvdata(dev);
gpiod_set_value_cansleep(p->enable_gpio, 0);
+
+ if (p->desc->delay.disable_to_power_off)
+ msleep(p->desc->delay.disable_to_power_off);
+
regulator_disable(p->supply);
p->unprepared_time = ktime_get();
@@ -407,6 +427,9 @@ static int panel_simple_prepare_once(struct panel_simple *p)
return err;
}
+ if (p->desc->delay.power_to_enable)
+ msleep(p->desc->delay.power_to_enable);
+
gpiod_set_value_cansleep(p->enable_gpio, 1);
delay = p->desc->delay.prepare;
@@ -782,6 +805,11 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,
break;
}
+ if (!panel->enable_gpio && desc->delay.disable_to_power_off)
+ dev_warn(dev, "Specify enable_gpio when using disable_to_power_off delay\n");
+ if (!panel->enable_gpio && desc->delay.power_to_enable)
+ dev_warn(dev, "Specify enable_gpio when using power_to_enable delay\n");
+
dev_set_drvdata(dev, panel);
/*
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [v5 3/5] drm/panel-simple: Support for delays between GPIO & regulator
2021-05-30 15:56 ` [v5 3/5] drm/panel-simple: Support for delays between GPIO & regulator Rajeev Nandan
@ 2021-06-03 0:05 ` Doug Anderson
0 siblings, 0 replies; 14+ messages in thread
From: Doug Anderson @ 2021-06-03 0:05 UTC (permalink / raw)
To: Rajeev Nandan
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Daniel Thompson, Krishna Manikandan, Sam Ravnborg, Jani Nikula,
linux-arm-msm, Abhinav Kumar, LKML, dri-devel, Andrzej Hajda,
Thierry Reding, Sean Paul, Laurent Pinchart, Kalyan Thota,
Kristian H. Kristensen, freedreno
Hi,
On Sun, May 30, 2021 at 8:57 AM Rajeev Nandan <rajeevny@codeaurora.org> wrote:
>
> Some panels datasheets may specify a delay between the enable GPIO and
> the regulator. Support this in panel-simple.
>
> Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
> ---
>
> Changes in v4:
> - New
>
> Changes in v5:
> - Update description (Douglas)
> - Warn if "power_to_enable" or "disable_to_power_off" is non-zero and panel->enable_gpio
> is NULL (Douglas)
>
> drivers/gpu/drm/panel/panel-simple.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index 047fad5..e3f5b7e 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -133,6 +133,22 @@ struct panel_desc {
> unsigned int prepare_to_enable;
>
> /**
> + * @delay.power_to_enable: Time for the power to enable the display on.
> + *
> + * The time (in milliseconds) to wait after powering up the display
> + * before asserting its enable pin.
> + */
> + unsigned int power_to_enable;
> +
> + /**
> + * @delay.disable_to_power_off: Time for the disable to power the display off.
> + *
> + * The time (in milliseconds) to wait before powering off the display
> + * after deasserting its enable pin.
> + */
> + unsigned int disable_to_power_off;
> +
> + /**
> * @delay.enable: Time for the panel to display a valid frame.
> *
> * The time (in milliseconds) that it takes for the panel to
> @@ -347,6 +363,10 @@ static int panel_simple_suspend(struct device *dev)
> struct panel_simple *p = dev_get_drvdata(dev);
>
> gpiod_set_value_cansleep(p->enable_gpio, 0);
> +
> + if (p->desc->delay.disable_to_power_off)
> + msleep(p->desc->delay.disable_to_power_off);
> +
> regulator_disable(p->supply);
> p->unprepared_time = ktime_get();
>
> @@ -407,6 +427,9 @@ static int panel_simple_prepare_once(struct panel_simple *p)
> return err;
> }
>
> + if (p->desc->delay.power_to_enable)
> + msleep(p->desc->delay.power_to_enable);
> +
> gpiod_set_value_cansleep(p->enable_gpio, 1);
>
> delay = p->desc->delay.prepare;
> @@ -782,6 +805,11 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc,
> break;
> }
>
> + if (!panel->enable_gpio && desc->delay.disable_to_power_off)
> + dev_warn(dev, "Specify enable_gpio when using disable_to_power_off delay\n");
> + if (!panel->enable_gpio && desc->delay.power_to_enable)
> + dev_warn(dev, "Specify enable_gpio when using power_to_enable delay\n");
Last nit is that the warning messages could be a little confusing to
someone reading the logs. I guess the target audience of the error
message is probably someone doing bringup. That person specified a
panel in their device tree and maybe isn't even aware that they're
using "disable_to_power_off" or "power_to_enable". Maybe wording
instead:
Need a delay after disabling panel GPIO, but a GPIO wasn't provided.
Need a delay after enabling panel GPIO, but a GPIO wasn't provided.
That's definitely getting into nittiness, though and I wouldn't be
upset if the patch landed with the existing messages. Thus, with or
without the change to the error message:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [v5 4/5] dt-bindings: display: simple: Add Samsung ATNA33XC20
2021-05-30 15:56 [v5 0/5] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20 Rajeev Nandan
` (2 preceding siblings ...)
2021-05-30 15:56 ` [v5 3/5] drm/panel-simple: Support for delays between GPIO & regulator Rajeev Nandan
@ 2021-05-30 15:56 ` Rajeev Nandan
2021-06-02 18:34 ` Rob Herring
2021-05-30 15:56 ` [v5 5/5] drm/panel-simple: " Rajeev Nandan
4 siblings, 1 reply; 14+ messages in thread
From: Rajeev Nandan @ 2021-05-30 15:56 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: daniel.thompson, Rajeev Nandan, mkrishn, jani.nikula,
linux-kernel, abhinavk, dianders, a.hajda, thierry.reding,
seanpaul, laurent.pinchart, kalyan_t, hoegsberg, sam
Add Samsung 13.3" FHD eDP AMOLED panel.
Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
(no changes since v4)
Changes in v4:
- New
Documentation/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 4a0a5e1..f5acfd6 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
@@ -242,6 +242,8 @@ properties:
- rocktech,rk101ii01d-ct
# Rocktech Display Ltd. RK070ER9427 800(RGB)x480 TFT LCD panel
- rocktech,rk070er9427
+ # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
+ - samsung,atna33xc20
# Samsung 12.2" (2560x1600 pixels) TFT LCD panel
- samsung,lsn122dl01-c01
# Samsung Electronics 10.1" WSVGA TFT LCD panel
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [v5 4/5] dt-bindings: display: simple: Add Samsung ATNA33XC20
2021-05-30 15:56 ` [v5 4/5] dt-bindings: display: simple: Add Samsung ATNA33XC20 Rajeev Nandan
@ 2021-06-02 18:34 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2021-06-02 18:34 UTC (permalink / raw)
To: Rajeev Nandan
Cc: devicetree, daniel.thompson, mkrishn, jani.nikula, linux-arm-msm,
dri-devel, linux-kernel, abhinavk, dianders, a.hajda,
thierry.reding, seanpaul, laurent.pinchart, kalyan_t, hoegsberg,
freedreno, sam
On Sun, 30 May 2021 21:26:11 +0530, Rajeev Nandan wrote:
> Add Samsung 13.3" FHD eDP AMOLED panel.
>
> Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> (no changes since v4)
>
> Changes in v4:
> - New
>
> Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [v5 5/5] drm/panel-simple: Add Samsung ATNA33XC20
2021-05-30 15:56 [v5 0/5] drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20 Rajeev Nandan
` (3 preceding siblings ...)
2021-05-30 15:56 ` [v5 4/5] dt-bindings: display: simple: Add Samsung ATNA33XC20 Rajeev Nandan
@ 2021-05-30 15:56 ` Rajeev Nandan
4 siblings, 0 replies; 14+ messages in thread
From: Rajeev Nandan @ 2021-05-30 15:56 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: daniel.thompson, Rajeev Nandan, mkrishn, jani.nikula,
linux-kernel, abhinavk, dianders, a.hajda, thierry.reding,
seanpaul, laurent.pinchart, kalyan_t, hoegsberg, sam
Add Samsung 13.3" FHD eDP AMOLED panel.
Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- New
Changes in v5:
- Remove "uses_dpcd_backlight" property, not required now. (Douglas)
drivers/gpu/drm/panel/panel-simple.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index e3f5b7e..ce60517 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -3562,6 +3562,36 @@ static const struct panel_desc rocktech_rk101ii01d_ct = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
};
+static const struct drm_display_mode samsung_atna33xc20_mode = {
+ .clock = 138770,
+ .hdisplay = 1920,
+ .hsync_start = 1920 + 48,
+ .hsync_end = 1920 + 48 + 32,
+ .htotal = 1920 + 48 + 32 + 80,
+ .vdisplay = 1080,
+ .vsync_start = 1080 + 8,
+ .vsync_end = 1080 + 8 + 8,
+ .vtotal = 1080 + 8 + 8 + 16,
+ .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC,
+};
+
+static const struct panel_desc samsung_atna33xc20 = {
+ .modes = &samsung_atna33xc20_mode,
+ .num_modes = 1,
+ .bpc = 10,
+ .size = {
+ .width = 294,
+ .height = 165,
+ },
+ .delay = {
+ .disable_to_power_off = 150,
+ .power_to_enable = 150,
+ .hpd_absent_delay = 200,
+ .unprepare = 500,
+ },
+ .connector_type = DRM_MODE_CONNECTOR_eDP,
+};
+
static const struct drm_display_mode samsung_lsn122dl01_c01_mode = {
.clock = 271560,
.hdisplay = 2560,
@@ -4563,6 +4593,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "rocktech,rk101ii01d-ct",
.data = &rocktech_rk101ii01d_ct,
}, {
+ .compatible = "samsung,atna33xc20",
+ .data = &samsung_atna33xc20,
+ }, {
.compatible = "samsung,lsn122dl01-c01",
.data = &samsung_lsn122dl01_c01,
}, {
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread