[PATCH 0/2] drm: sun4i/dsi: allow modeset helpers to manage display

[PATCH 0/2] drm: sun4i/dsi: allow modeset helpers to manage display

[Brandon Cheo Fusi]

This change moves DSI PHY poweron/off from the encoder to the TCON. 

As a consequence enabling or disabling the DSI sink can be left to the modeset
helpers, and bridge support easily introduced without touching the 
drm_encoder.bridge_chain or converting the encoder to a drm_bridge.

Brandon Cheo Fusi (2):
  drm: sun4i/dsi: factor out DSI PHY poweron and poweroff
  drm: sun4i: tie DSI PHY Poweron/off to crtc enable/disable

 drivers/gpu/drm/sun4i/sun4i_tcon.c     | 10 +++-
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 64 ++++++++++++--------------
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  4 ++
 3 files changed, 43 insertions(+), 35 deletions(-)

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] drm: sun4i/dsi: factor out DSI PHY poweron and poweroff

[Brandon Cheo Fusi]

Factor out PHY poweron and poweroff sequences from sun6i_dsi_encoder_enable
and sun6i_dsi_encoder_disable.This leaves nothing to be be done in
sun6i_dsi_encoder_disable, so get rid of that. Also remove
drm_panel_<prepare/enable/disable/unprepare> as these would be invoked the
modeset helpers.

Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 64 ++++++++++++--------------
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  4 ++
 2 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 760ff05ea..4dc92109e 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -713,7 +713,7 @@ static int sun6i_dsi_start(struct sun6i_dsi *dsi,
 	return 0;
 }
 
-static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
+void sun6i_dsi_phy_power_on(const struct drm_encoder *encoder)
 {
 	struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
 	struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
@@ -768,43 +768,12 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
 	phy_set_mode(dsi->dphy, PHY_MODE_MIPI_DPHY);
 	phy_configure(dsi->dphy, &opts);
 	phy_power_on(dsi->dphy);
-
-	if (dsi->panel)
-		drm_panel_prepare(dsi->panel);
-
-	/*
-	 * FIXME: This should be moved after the switch to HS mode.
-	 *
-	 * Unfortunately, once in HS mode, it seems like we're not
-	 * able to send DCS commands anymore, which would prevent any
-	 * panel to send any DCS command as part as their enable
-	 * method, which is quite common.
-	 *
-	 * I haven't seen any artifact due to that sub-optimal
-	 * ordering on the panels I've tested it with, so I guess this
-	 * will do for now, until that IP is better understood.
-	 */
-	if (dsi->panel)
-		drm_panel_enable(dsi->panel);
-
-	sun6i_dsi_start(dsi, DSI_START_HSC);
-
-	udelay(1000);
-
-	sun6i_dsi_start(dsi, DSI_START_HSD);
 }
 
-static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
+void sun6i_dsi_phy_power_off(const struct drm_encoder *encoder)
 {
 	struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
 
-	DRM_DEBUG_DRIVER("Disabling DSI output\n");
-
-	if (dsi->panel) {
-		drm_panel_disable(dsi->panel);
-		drm_panel_unprepare(dsi->panel);
-	}
-
 	phy_power_off(dsi->dphy);
 	phy_exit(dsi->dphy);
 
@@ -813,6 +782,34 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
 	regulator_disable(dsi->regulator);
 }
 
+static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder, struct drm_atomic_state *old_state)
+{
+	struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
+
+	DRM_DEBUG_DRIVER("Enabling DSI output\n");
+
+	sun6i_dsi_start(dsi, DSI_START_HSC);
+
+	udelay(1000);
+
+	sun6i_dsi_start(dsi, DSI_START_HSD);
+
+	/*
+	 * NOTE
+	 *
+	 * Unfortunately, once in HS mode, it seems like we're not
+	 * able to send DCS commands anymore, which would prevent any
+	 * panel to send any DCS command as part as their enable
+	 * method, which is quite common.
+	 *
+	 * So maybe panels/bridges should send any init DCS commands in their
+	 * prepare/pre_enable methods? This should work as the DSI PHY is active
+	 * before those hooks are called.
+	 *
+	 * This will do for now, until that IP is better understood.
+	 */
+}
+
 static int sun6i_dsi_get_modes(struct drm_connector *connector)
 {
 	struct sun6i_dsi *dsi = connector_to_sun6i_dsi(connector);
@@ -843,7 +840,6 @@ static const struct drm_connector_funcs sun6i_dsi_connector_funcs = {
 };
 
 static const struct drm_encoder_helper_funcs sun6i_dsi_enc_helper_funcs = {
-	.disable	= sun6i_dsi_encoder_disable,
 	.enable		= sun6i_dsi_encoder_enable,
 };
 
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index f1ddefe0f..a0b541f48 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -40,6 +40,10 @@ struct sun6i_dsi {
 	const struct sun6i_dsi_variant *variant;
 };
 
+void sun6i_dsi_phy_power_on(const struct drm_encoder *encoder);
+
+void sun6i_dsi_phy_power_off(const struct drm_encoder *encoder);
+
 static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
 {
 	return container_of(host, struct sun6i_dsi, host);
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] drm: sun4i/dsi: factor out DSI PHY poweron and poweroff

[kernel test robot]

Hi Brandon,

kernel test robot noticed the following build errors:

[auto build test ERROR on sunxi/sunxi/for-next]
[also build test ERROR on linus/master v6.3-rc6 next-20230406]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Brandon-Cheo-Fusi/drm-sun4i-dsi-factor-out-DSI-PHY-poweron-and-poweroff/20230410-165257
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
patch link:    https://lore.kernel.org/r/20230410084750.164016-2-fusibrandon13%40gmail.com
patch subject: [PATCH 1/2] drm: sun4i/dsi: factor out DSI PHY poweron and poweroff
config: arm64-randconfig-r002-20230410 (https://download.01.org/0day-ci/archive/20230411/202304110053.n5nu03YZ-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 2c57868e2e877f73c339796c3374ae660bb77f0d)
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/intel-lab-lkp/linux/commit/afa9cb6821e4527f07c10a777ea44e380b524858
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Brandon-Cheo-Fusi/drm-sun4i-dsi-factor-out-DSI-PHY-poweron-and-poweroff/20230410-165257
        git checkout afa9cb6821e4527f07c10a777ea44e380b524858
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/gpu/drm/sun4i/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304110053.n5nu03YZ-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c:843:13: error: incompatible function pointer types initializing 'void (*)(struct drm_encoder *)' with an expression of type 'void (struct drm_encoder *, struct drm_atomic_state *)' [-Wincompatible-function-pointer-types]
           .enable         = sun6i_dsi_encoder_enable,
                             ^~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +843 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c

133add5b5ad42b Maxime Ripard 2018-04-04  841  
133add5b5ad42b Maxime Ripard 2018-04-04  842  static const struct drm_encoder_helper_funcs sun6i_dsi_enc_helper_funcs = {
133add5b5ad42b Maxime Ripard 2018-04-04 @843  	.enable		= sun6i_dsi_encoder_enable,
133add5b5ad42b Maxime Ripard 2018-04-04  844  };
133add5b5ad42b Maxime Ripard 2018-04-04  845  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] drm: sun4i/dsi: factor out DSI PHY poweron and poweroff

[kernel test robot]

Hi Brandon,

kernel test robot noticed the following build errors:

[auto build test ERROR on sunxi/sunxi/for-next]
[also build test ERROR on linus/master v6.3-rc6 next-20230406]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Brandon-Cheo-Fusi/drm-sun4i-dsi-factor-out-DSI-PHY-poweron-and-poweroff/20230410-165257
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
patch link:    https://lore.kernel.org/r/20230410084750.164016-2-fusibrandon13%40gmail.com
patch subject: [PATCH 1/2] drm: sun4i/dsi: factor out DSI PHY poweron and poweroff
config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230411/202304110110.ZLinpepn-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.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/intel-lab-lkp/linux/commit/afa9cb6821e4527f07c10a777ea44e380b524858
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Brandon-Cheo-Fusi/drm-sun4i-dsi-factor-out-DSI-PHY-poweron-and-poweroff/20230410-165257
        git checkout afa9cb6821e4527f07c10a777ea44e380b524858
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/gpu/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304110110.ZLinpepn-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c:843:27: error: initialization of 'void (*)(struct drm_encoder *)' from incompatible pointer type 'void (*)(struct drm_encoder *, struct drm_atomic_state *)' [-Werror=incompatible-pointer-types]
     843 |         .enable         = sun6i_dsi_encoder_enable,
         |                           ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c:843:27: note: (near initialization for 'sun6i_dsi_enc_helper_funcs.enable')
   cc1: some warnings being treated as errors


vim +843 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c

133add5b5ad42b Maxime Ripard 2018-04-04  841  
133add5b5ad42b Maxime Ripard 2018-04-04  842  static const struct drm_encoder_helper_funcs sun6i_dsi_enc_helper_funcs = {
133add5b5ad42b Maxime Ripard 2018-04-04 @843  	.enable		= sun6i_dsi_encoder_enable,
133add5b5ad42b Maxime Ripard 2018-04-04  844  };
133add5b5ad42b Maxime Ripard 2018-04-04  845  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] drm: sun4i: tie DSI PHY Poweron/off to crtc enable/disable

[Brandon Cheo Fusi]

Poweron/off the DSI PHY when the crtc is enabled/disabled. This allows the modeset
helpers to manage the DSI sink while preserving the old drm_panel_<prepare,enable>
and drm_panel_<disable,unprepare> sequences.

Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 523a6d787..6f50dc66a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -190,7 +190,7 @@ void sun4i_tcon_set_status(struct sun4i_tcon *tcon,
 			   const struct drm_encoder *encoder,
 			   bool enabled)
 {
-	bool is_lvds = false;
+	bool is_lvds = false, is_dsi = false;
 	int channel;
 
 	switch (encoder->encoder_type) {
@@ -198,6 +198,8 @@ void sun4i_tcon_set_status(struct sun4i_tcon *tcon,
 		is_lvds = true;
 		fallthrough;
 	case DRM_MODE_ENCODER_DSI:
+		is_dsi = true;
+		fallthrough;
 	case DRM_MODE_ENCODER_NONE:
 		channel = 0;
 		break;
@@ -221,6 +223,12 @@ void sun4i_tcon_set_status(struct sun4i_tcon *tcon,
 		sun4i_tcon_lvds_set_status(tcon, encoder, true);
 
 	sun4i_tcon_channel_set_status(tcon, channel, enabled);
+
+	if (is_dsi) {
+		/* turn DSI phy on or off */
+		(enabled) ? sun6i_dsi_phy_power_on(encoder)
+				  : sun6i_dsi_phy_power_off(encoder);
+	}
 }
 
 void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable)
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] drm: sun4i/dsi: allow modeset helpers to manage display

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 691 bytes --]

Hi,

On Mon, Apr 10, 2023 at 09:47:48AM +0100, Brandon Cheo Fusi wrote:
> This change moves DSI PHY poweron/off from the encoder to the TCON. 
> 
> As a consequence enabling or disabling the DSI sink can be left to the
> modeset helpers, and bridge support easily introduced without touching
> the drm_encoder.bridge_chain or converting the encoder to a
> drm_bridge.

I guess this is related to the discussion we had a couple of weeks ago
about being able to send DCS commands while in HS mode.

This just seems to workaround the issue entirely, breaking KMS
abstractions in the process. We had some leads on how to fix it
properly, so please follow them instead.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel