Re: [Intel-gfx] [PATCH v3 14/15] drm/i915/dsi: Replace poking of CHV GPIOs behind the driver's back
Hi Andy,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-intel/for-linux-next-fixes linus/master v6.6
next-20231103]
[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/Andy-Shevchenko/drm-i915-dsi-assume-BXT-gpio-works-for-non-native-GPIO/20231103-064642
base: git://anongit.freedesktop.org/drm-intel for-linux-next
patch link:
https://lore.kernel.org/r/20231102151228.668842-15-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v3 14/15] drm/i915/dsi: Replace poking of CHV GPIOs
behind the driver's back
config: x86_64-rhel-8.3-rust
(https://download.01.org/0day-ci/archive/20231104/202311040312.tf6btkw0-...@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git
ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20231104/202311040312.tf6btkw0-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202311040312.tf6btkw0-...@intel.com/
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/intel_dsi_vbt.c:272:4: error: call to
>> undeclared function 'soc_exec_opaque_gpio'; ISO C99 and later do not support
>> implicit function declarations [-Wimplicit-function-declaration]
soc_exec_opaque_gpio(connector, gpio_index,
"INT33FF:03", "Panel SE",
^
drivers/gpu/drm/i915/display/intel_dsi_vbt.c:275:4: error: call to
undeclared function 'soc_exec_opaque_gpio'; ISO C99 and later do not support
implicit function declarations [-Wimplicit-function-declaration]
soc_exec_opaque_gpio(connector, gpio_index,
"INT33FF:00", "Panel SW",
^
drivers/gpu/drm/i915/display/intel_dsi_vbt.c:278:4: error: call to
undeclared function 'soc_exec_opaque_gpio'; ISO C99 and later do not support
implicit function declarations [-Wimplicit-function-declaration]
soc_exec_opaque_gpio(connector, gpio_index,
"INT33FF:02", "Panel E",
^
drivers/gpu/drm/i915/display/intel_dsi_vbt.c:281:4: error: call to
undeclared function 'soc_exec_opaque_gpio'; ISO C99 and later do not support
implicit function declarations [-Wimplicit-function-declaration]
soc_exec_opaque_gpio(connector, gpio_index,
"INT33FF:01", "Panel N",
^
drivers/gpu/drm/i915/display/intel_dsi_vbt.c:299:3: error: call to
undeclared function 'soc_exec_opaque_gpio'; ISO C99 and later do not support
implicit function declarations [-Wimplicit-function-declaration]
soc_exec_opaque_gpio(connector, gpio_index, "INT33FF:01",
"Panel N",
^
5 errors generated.
vim +/soc_exec_opaque_gpio +272 drivers/gpu/drm/i915/display/intel_dsi_vbt.c
263
264 static void chv_gpio_set_value(struct intel_connector *connector,
265 u8 gpio_source, u8 gpio_index, bool
value)
266 {
267 struct drm_i915_private *dev_priv =
to_i915(connector->base.dev);
268
269 if (connector->panel.vbt.dsi.seq_version >= 3) {
270 if (gpio_index >= CHV_GPIO_IDX_START_SE) {
271 /* XXX: it's unclear whether 255->57 is part of
SE. */
> 272 soc_exec_opaque_gpio(connector, gpio_index,
> "INT33FF:03", "Panel SE",
273 gpio_index -
CHV_GPIO_IDX_START_SW, value);
274 } else if (gpio_index >= CHV_GPIO_IDX_START_SW) {
275 soc_exec_opaque_gpio(connector, gpio_index,
"INT33FF:00", "Panel SW",
276 gpio_index -
CHV_GPIO_IDX_START_SW, value);
277 } else if (gpio_index >= CHV_GPIO_IDX_START_E) {
278 soc_exec_opaque_gpio(connector, gpio_index,
"INT33FF:02", "Panel E",
279 gpio_index -
CHV_GPIO_IDX_START_E, value);
280 } else {
281 soc_exec_opaque_gpio(connector, gpio_index,
"INT33FF:01", "Panel N",
282 gpio_index -
CHV_GPIO_IDX_START_N, value);
283 }
284 } else {
285 /* XXX: The spec is unclear about CHV GPIO on seq v2 */
286 if (gpio_source != 0) {
287 drm_dbg_kms(&dev_priv->drm,
28
Re: [Intel-gfx] [PATCH v3 14/15] drm/i915/dsi: Replace poking of CHV GPIOs behind the driver's back
On Thu, Nov 02, 2023 at 04:47:41PM +0100, Hans de Goede wrote: > On 11/2/23 16:12, Andy Shevchenko wrote: ... > > + soc_exec_opaque_gpio(connector, gpio_index, > > "INT33FF:03", "Panel SE", > > +gpio_index - > > CHV_GPIO_IDX_START_SW, value); > > The "gpio_index - CHV_GPIO_IDX_START_SW" here needs to be "gpio_index - > CHV_GPIO_IDX_START_SE". > > Also this patch needs s/soc_exec_opaque_gpio/soc_opaque_gpio_set_value/ to > compile ... Ah, indeed. I looks like I run the test build, but forgot to look into the result. :-( -- With Best Regards, Andy Shevchenko
Re: [Intel-gfx] [PATCH v3 14/15] drm/i915/dsi: Replace poking of CHV GPIOs behind the driver's back
Hi,
On 11/2/23 16:12, Andy Shevchenko wrote:
> It's a dirty hack in the driver that pokes GPIO registers behind
> the driver's back. Moreoever it might be problematic as simultaneous
> I/O may hang the system, see the commit 0bd50d719b00 ("pinctrl:
> cherryview: prevent concurrent access to GPIO controllers") for
> the details. Taking all this into consideration replace the hack
> with proper GPIO APIs being used.
>
> Signed-off-by: Andy Shevchenko
> ---
> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 47 +---
> 1 file changed, 10 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index b1736c1301ea..ffc65c943b11 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -66,19 +66,6 @@ struct i2c_adapter_lookup {
> #define CHV_GPIO_IDX_START_SW100
> #define CHV_GPIO_IDX_START_SE198
>
> -#define CHV_VBT_MAX_PINS_PER_FMLY15
> -
> -#define CHV_GPIO_PAD_CFG0(f, i) (0x4400 + (f) * 0x400 + (i) * 8)
> -#define CHV_GPIO_GPIOEN (1 << 15)
> -#define CHV_GPIO_GPIOCFG_GPIO (0 << 8)
> -#define CHV_GPIO_GPIOCFG_GPO(1 << 8)
> -#define CHV_GPIO_GPIOCFG_GPI(2 << 8)
> -#define CHV_GPIO_GPIOCFG_HIZ(3 << 8)
> -#define CHV_GPIO_GPIOTXSTATE(state) ((!!(state)) << 1)
> -
> -#define CHV_GPIO_PAD_CFG1(f, i) (0x4400 + (f) * 0x400 + (i) * 8
> + 4)
> -#define CHV_GPIO_CFGLOCK(1 << 31)
> -
> /* ICL DSI Display GPIO Pins */
> #define ICL_GPIO_DDSP_HPD_A 0
> #define ICL_GPIO_L_VDDEN_1 1
> @@ -278,23 +265,21 @@ static void chv_gpio_set_value(struct intel_connector
> *connector,
> u8 gpio_source, u8 gpio_index, bool value)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> - u16 cfg0, cfg1;
> - u16 family_num;
> - u8 port;
>
> if (connector->panel.vbt.dsi.seq_version >= 3) {
> if (gpio_index >= CHV_GPIO_IDX_START_SE) {
> /* XXX: it's unclear whether 255->57 is part of SE. */
> - gpio_index -= CHV_GPIO_IDX_START_SE;
> - port = CHV_IOSF_PORT_GPIO_SE;
> + soc_exec_opaque_gpio(connector, gpio_index,
> "INT33FF:03", "Panel SE",
> + gpio_index -
> CHV_GPIO_IDX_START_SW, value);
The "gpio_index - CHV_GPIO_IDX_START_SW" here needs to be "gpio_index -
CHV_GPIO_IDX_START_SE".
Also this patch needs s/soc_exec_opaque_gpio/soc_opaque_gpio_set_value/ to
compile ...
Regards,
Hans
> } else if (gpio_index >= CHV_GPIO_IDX_START_SW) {
> - gpio_index -= CHV_GPIO_IDX_START_SW;
> - port = CHV_IOSF_PORT_GPIO_SW;
> + soc_exec_opaque_gpio(connector, gpio_index,
> "INT33FF:00", "Panel SW",
> + gpio_index -
> CHV_GPIO_IDX_START_SW, value);
> } else if (gpio_index >= CHV_GPIO_IDX_START_E) {
> - gpio_index -= CHV_GPIO_IDX_START_E;
> - port = CHV_IOSF_PORT_GPIO_E;
> + soc_exec_opaque_gpio(connector, gpio_index,
> "INT33FF:02", "Panel E",
> + gpio_index - CHV_GPIO_IDX_START_E,
> value);
> } else {
> - port = CHV_IOSF_PORT_GPIO_N;
> + soc_exec_opaque_gpio(connector, gpio_index,
> "INT33FF:01", "Panel N",
> + gpio_index - CHV_GPIO_IDX_START_N,
> value);
> }
> } else {
> /* XXX: The spec is unclear about CHV GPIO on seq v2 */
> @@ -311,21 +296,9 @@ static void chv_gpio_set_value(struct intel_connector
> *connector,
> return;
> }
>
> - port = CHV_IOSF_PORT_GPIO_N;
> + soc_exec_opaque_gpio(connector, gpio_index, "INT33FF:01",
> "Panel N",
> + gpio_index - CHV_GPIO_IDX_START_N, value);
> }
> -
> - family_num = gpio_index / CHV_VBT_MAX_PINS_PER_FMLY;
> - gpio_index = gpio_index % CHV_VBT_MAX_PINS_PER_FMLY;
> -
> - cfg0 = CHV_GPIO_PAD_CFG0(family_num, gpio_index);
> - cfg1 = CHV_GPIO_PAD_CFG1(family_num, gpio_index);
> -
> - vlv_iosf_sb_get(dev_priv, BIT(VLV_IOSF_SB_GPIO));
> - vlv_iosf_sb_write(dev_priv, port, cfg1, 0);
> - vlv_iosf_sb_write(dev_priv, port, cfg0,
> - CHV_GPIO_GPIOEN | CHV_GPIO_GPIOCFG_GPO |
> - CHV_GPIO_GPIOTXSTATE(value));
> - vlv_iosf_sb_put(dev_priv, BIT(VLV_IOSF_SB_GPIO));
> }
>
> static void bxt_gpio_set_value(struct intel_connector *connector,
