| Message ID | 20231123140244.4183869-2-mitulkumar.ajitkumar.golani@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable Adaptive Sync SDP Support for DP | expand |
On 11/23/2023 7:32 PM, Mitul Golani wrote: > Add structure representing Adaptive Sync Secondary Data > Packet (AS SDP). Also, add Adaptive Sync SDP logging in > drm_dp_helper.c to facilitate debugging. > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > --- > drivers/gpu/drm/display/drm_dp_helper.c | 15 +++++++++++++ > include/drm/display/drm_dp.h | 1 + > include/drm/display/drm_dp_helper.h | 30 +++++++++++++++++++++++++ > 3 files changed, 46 insertions(+) > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c > index d72b6f9a352c..a205e14a6681 100644 > --- a/drivers/gpu/drm/display/drm_dp_helper.c > +++ b/drivers/gpu/drm/display/drm_dp_helper.c > @@ -2917,6 +2917,21 @@ void drm_dp_vsc_sdp_log(const char *level, struct device *dev, > } > EXPORT_SYMBOL(drm_dp_vsc_sdp_log); > > +void drm_dp_as_sdp_log(const char *level, struct device *dev, > + const struct drm_dp_as_sdp *async) Perhaps as_sdp, instead of async to avoid confusion? > +{ > +#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__) > + DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC", I think you mean AS_SDP here. Also, you need to send this to dri-devel as well. Regards, Ankit > + async->revision, async->length); > + DP_SDP_LOG(" vmin: %d vmax: %d\n", async->vmin, async->vmax); > + DP_SDP_LOG(" target_rr: %s\n", async->target_rr); > + DP_SDP_LOG(" duration_incr_ms: %u\n", async->duration_incr_ms); > + DP_SDP_LOG(" duration_decr_ms: %u\n", async->duration_decr_ms); > + DP_SDP_LOG(" operation_mode: %u\n", async->operation_mode); > +#undef DP_SDP_LOG > +} > +EXPORT_SYMBOL(drm_dp_as_sdp_log); > + > /** > * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON > * @dpcd: DisplayPort configuration data > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 83d2039c018b..0575ab8ea088 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -1578,6 +1578,7 @@ enum drm_dp_phy { > #define DP_SDP_PPS 0x10 /* DP 1.4 */ > #define DP_SDP_VSC_EXT_VESA 0x20 /* DP 1.4 */ > #define DP_SDP_VSC_EXT_CEA 0x21 /* DP 1.4 */ > +#define DP_SDP_ADAPTIVE_SYNC 0x22 /* DP 1.4 */ > /* 0x80+ CEA-861 infoframe types */ > > #define DP_SDP_AUDIO_INFOFRAME_HB2 0x1b > diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h > index 863b2e7add29..63b6bef3f21d 100644 > --- a/include/drm/display/drm_dp_helper.h > +++ b/include/drm/display/drm_dp_helper.h > @@ -98,6 +98,36 @@ struct drm_dp_vsc_sdp { > enum dp_content_type content_type; > }; > > +/** > + * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP > + * > + * This structure represents a DP AS SDP of drm > + * It is based on DP 2.1 spec [Table 2-126: Adaptive-Sync SDP Header Bytes] and > + * [Table 2-127: Adaptive-Sync SDP Payload for DB0 through DB8] > + * > + * @sdp_type: secondary-data packet type > + * @length: number of valid data bytes > + * @vmin: minimum vtotal > + * @vmax: maximum vtotal > + * @duration_incr_ms: Successive frame duration increase > + * @duration_decr_ms: Successive frame duration decrease > + * @operation_mode: Adaptive Sync Operation Mode > + */ > + > +struct drm_dp_as_sdp { > + unsigned char sdp_type; > + unsigned char revision; > + unsigned char length; > + u16 vmin, vmax; > + u16 target_rr; > + u8 duration_incr_ms; > + u8 duration_decr_ms; > + u8 operation_mode; > +}; > + > +void drm_dp_as_sdp_log(const char *level, struct device *dev, > + const struct drm_dp_as_sdp *async); > + > void drm_dp_vsc_sdp_log(const char *level, struct device *dev, > const struct drm_dp_vsc_sdp *vsc); >
Hi Mitul,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-tip/drm-tip]
url: https://github.com/intel-lab-lkp/linux/commits/Mitul-Golani/drm-Add-Adaptive-Sync-SDP-logging/20231123-220931
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link: https://lore.kernel.org/r/20231123140244.4183869-2-mitulkumar.ajitkumar.golani%40intel.com
patch subject: [Intel-gfx] [PATCH 1/3] drm: Add Adaptive Sync SDP logging
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20231124/202311241136.tl5STYqx-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231124/202311241136.tl5STYqx-lkp@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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311241136.tl5STYqx-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/backlight.h:12,
from drivers/gpu/drm/display/drm_dp_helper.c:23:
drivers/gpu/drm/display/drm_dp_helper.c: In function 'drm_dp_as_sdp_log':
>> drivers/gpu/drm/display/drm_dp_helper.c:2927:20: warning: format '%s' expects argument of type 'char *', but argument 4 has type 'int' [-Wformat=]
2927 | DP_SDP_LOG(" target_rr: %s\n", async->target_rr);
| ^~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~
| |
| int
include/linux/dev_printk.h:129:41: note: in definition of macro 'dev_printk'
129 | _dev_printk(level, dev, fmt, ##__VA_ARGS__); \
| ^~~
drivers/gpu/drm/display/drm_dp_helper.c:2927:9: note: in expansion of macro 'DP_SDP_LOG'
2927 | DP_SDP_LOG(" target_rr: %s\n", async->target_rr);
| ^~~~~~~~~~
drivers/gpu/drm/display/drm_dp_helper.c:2927:37: note: format string is defined here
2927 | DP_SDP_LOG(" target_rr: %s\n", async->target_rr);
| ~^
| |
| char *
| %d
vim +2927 drivers/gpu/drm/display/drm_dp_helper.c
2919
2920 void drm_dp_as_sdp_log(const char *level, struct device *dev,
2921 const struct drm_dp_as_sdp *async)
2922 {
2923 #define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
2924 DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC",
2925 async->revision, async->length);
2926 DP_SDP_LOG(" vmin: %d vmax: %d\n", async->vmin, async->vmax);
> 2927 DP_SDP_LOG(" target_rr: %s\n", async->target_rr);
2928 DP_SDP_LOG(" duration_incr_ms: %u\n", async->duration_incr_ms);
2929 DP_SDP_LOG(" duration_decr_ms: %u\n", async->duration_decr_ms);
2930 DP_SDP_LOG(" operation_mode: %u\n", async->operation_mode);
2931 #undef DP_SDP_LOG
2932 }
2933 EXPORT_SYMBOL(drm_dp_as_sdp_log);
2934
Hi Mitul, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-tip/drm-tip] url: https://github.com/intel-lab-lkp/linux/commits/Mitul-Golani/drm-Add-Adaptive-Sync-SDP-logging/20231123-220931 base: git://anongit.freedesktop.org/drm/drm-tip drm-tip patch link: https://lore.kernel.org/r/20231123140244.4183869-2-mitulkumar.ajitkumar.golani%40intel.com patch subject: [Intel-gfx] [PATCH 1/3] drm: Add Adaptive Sync SDP logging config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231124/202311241251.ZXIZrJRp-lkp@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/20231124/202311241251.ZXIZrJRp-lkp@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 <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202311241251.ZXIZrJRp-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpu/drm/display/drm_dp_helper.c:2927:36: warning: format specifies type 'char *' but the argument has type 'u16' (aka 'unsigned short') [-Wformat] DP_SDP_LOG(" target_rr: %s\n", async->target_rr); ~~ ^~~~~~~~~~~~~~~~ %hu drivers/gpu/drm/display/drm_dp_helper.c:2923:60: note: expanded from macro 'DP_SDP_LOG' #define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__) ~~~ ^~~~~~~~~~~ include/linux/dev_printk.h:129:34: note: expanded from macro 'dev_printk' _dev_printk(level, dev, fmt, ##__VA_ARGS__); \ ~~~ ^~~~~~~~~~~ 1 warning generated. vim +2927 drivers/gpu/drm/display/drm_dp_helper.c 2919 2920 void drm_dp_as_sdp_log(const char *level, struct device *dev, 2921 const struct drm_dp_as_sdp *async) 2922 { 2923 #define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__) 2924 DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC", 2925 async->revision, async->length); 2926 DP_SDP_LOG(" vmin: %d vmax: %d\n", async->vmin, async->vmax); > 2927 DP_SDP_LOG(" target_rr: %s\n", async->target_rr); 2928 DP_SDP_LOG(" duration_incr_ms: %u\n", async->duration_incr_ms); 2929 DP_SDP_LOG(" duration_decr_ms: %u\n", async->duration_decr_ms); 2930 DP_SDP_LOG(" operation_mode: %u\n", async->operation_mode); 2931 #undef DP_SDP_LOG 2932 } 2933 EXPORT_SYMBOL(drm_dp_as_sdp_log); 2934
On Thu, 23 Nov 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote: > Add structure representing Adaptive Sync Secondary Data > Packet (AS SDP). Also, add Adaptive Sync SDP logging in > drm_dp_helper.c to facilitate debugging. > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > --- > drivers/gpu/drm/display/drm_dp_helper.c | 15 +++++++++++++ > include/drm/display/drm_dp.h | 1 + > include/drm/display/drm_dp_helper.h | 30 +++++++++++++++++++++++++ > 3 files changed, 46 insertions(+) > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c > index d72b6f9a352c..a205e14a6681 100644 > --- a/drivers/gpu/drm/display/drm_dp_helper.c > +++ b/drivers/gpu/drm/display/drm_dp_helper.c > @@ -2917,6 +2917,21 @@ void drm_dp_vsc_sdp_log(const char *level, struct device *dev, > } > EXPORT_SYMBOL(drm_dp_vsc_sdp_log); > > +void drm_dp_as_sdp_log(const char *level, struct device *dev, > + const struct drm_dp_as_sdp *async) > +{ > +#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__) > + DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC", > + async->revision, async->length); > + DP_SDP_LOG(" vmin: %d vmax: %d\n", async->vmin, async->vmax); > + DP_SDP_LOG(" target_rr: %s\n", async->target_rr); > + DP_SDP_LOG(" duration_incr_ms: %u\n", async->duration_incr_ms); > + DP_SDP_LOG(" duration_decr_ms: %u\n", async->duration_decr_ms); > + DP_SDP_LOG(" operation_mode: %u\n", async->operation_mode); > +#undef DP_SDP_LOG > +} > +EXPORT_SYMBOL(drm_dp_as_sdp_log); This inspired me to resurrect a drm logging series I've had [1], in particular patch [2]. Please let's do this properly. Also, throughout the series, please don't use "async" for "adaptive sync". It's bound to confuse people. Async means asynchronous, period. [1] https://patchwork.freedesktop.org/series/126873/ [2] https://patchwork.freedesktop.org/patch/msgid/95f1e3981fa3c5304f3c74e82330f12983d35735.1700829750.git.jani.nikula@intel.com > + > /** > * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON > * @dpcd: DisplayPort configuration data > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 83d2039c018b..0575ab8ea088 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -1578,6 +1578,7 @@ enum drm_dp_phy { > #define DP_SDP_PPS 0x10 /* DP 1.4 */ > #define DP_SDP_VSC_EXT_VESA 0x20 /* DP 1.4 */ > #define DP_SDP_VSC_EXT_CEA 0x21 /* DP 1.4 */ > +#define DP_SDP_ADAPTIVE_SYNC 0x22 /* DP 1.4 */ This is completely unrelated to everything else in the patch. > /* 0x80+ CEA-861 infoframe types */ > > #define DP_SDP_AUDIO_INFOFRAME_HB2 0x1b > diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h > index 863b2e7add29..63b6bef3f21d 100644 > --- a/include/drm/display/drm_dp_helper.h > +++ b/include/drm/display/drm_dp_helper.h > @@ -98,6 +98,36 @@ struct drm_dp_vsc_sdp { > enum dp_content_type content_type; > }; > > +/** > + * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP > + * > + * This structure represents a DP AS SDP of drm > + * It is based on DP 2.1 spec [Table 2-126: Adaptive-Sync SDP Header Bytes] and > + * [Table 2-127: Adaptive-Sync SDP Payload for DB0 through DB8] > + * > + * @sdp_type: secondary-data packet type > + * @length: number of valid data bytes > + * @vmin: minimum vtotal > + * @vmax: maximum vtotal > + * @duration_incr_ms: Successive frame duration increase > + * @duration_decr_ms: Successive frame duration decrease > + * @operation_mode: Adaptive Sync Operation Mode > + */ > + > +struct drm_dp_as_sdp { > + unsigned char sdp_type; > + unsigned char revision; > + unsigned char length; Why unsigned char here... > + u16 vmin, vmax; > + u16 target_rr; > + u8 duration_incr_ms; > + u8 duration_decr_ms; > + u8 operation_mode; ...but u8 here? > +}; > + > +void drm_dp_as_sdp_log(const char *level, struct device *dev, > + const struct drm_dp_as_sdp *async); > + > void drm_dp_vsc_sdp_log(const char *level, struct device *dev, > const struct drm_dp_vsc_sdp *vsc);
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index d72b6f9a352c..a205e14a6681 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -2917,6 +2917,21 @@ void drm_dp_vsc_sdp_log(const char *level, struct device *dev, } EXPORT_SYMBOL(drm_dp_vsc_sdp_log); +void drm_dp_as_sdp_log(const char *level, struct device *dev, + const struct drm_dp_as_sdp *async) +{ +#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__) + DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC", + async->revision, async->length); + DP_SDP_LOG(" vmin: %d vmax: %d\n", async->vmin, async->vmax); + DP_SDP_LOG(" target_rr: %s\n", async->target_rr); + DP_SDP_LOG(" duration_incr_ms: %u\n", async->duration_incr_ms); + DP_SDP_LOG(" duration_decr_ms: %u\n", async->duration_decr_ms); + DP_SDP_LOG(" operation_mode: %u\n", async->operation_mode); +#undef DP_SDP_LOG +} +EXPORT_SYMBOL(drm_dp_as_sdp_log); + /** * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON * @dpcd: DisplayPort configuration data diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 83d2039c018b..0575ab8ea088 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -1578,6 +1578,7 @@ enum drm_dp_phy { #define DP_SDP_PPS 0x10 /* DP 1.4 */ #define DP_SDP_VSC_EXT_VESA 0x20 /* DP 1.4 */ #define DP_SDP_VSC_EXT_CEA 0x21 /* DP 1.4 */ +#define DP_SDP_ADAPTIVE_SYNC 0x22 /* DP 1.4 */ /* 0x80+ CEA-861 infoframe types */ #define DP_SDP_AUDIO_INFOFRAME_HB2 0x1b diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h index 863b2e7add29..63b6bef3f21d 100644 --- a/include/drm/display/drm_dp_helper.h +++ b/include/drm/display/drm_dp_helper.h @@ -98,6 +98,36 @@ struct drm_dp_vsc_sdp { enum dp_content_type content_type; }; +/** + * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP + * + * This structure represents a DP AS SDP of drm + * It is based on DP 2.1 spec [Table 2-126: Adaptive-Sync SDP Header Bytes] and + * [Table 2-127: Adaptive-Sync SDP Payload for DB0 through DB8] + * + * @sdp_type: secondary-data packet type + * @length: number of valid data bytes + * @vmin: minimum vtotal + * @vmax: maximum vtotal + * @duration_incr_ms: Successive frame duration increase + * @duration_decr_ms: Successive frame duration decrease + * @operation_mode: Adaptive Sync Operation Mode + */ + +struct drm_dp_as_sdp { + unsigned char sdp_type; + unsigned char revision; + unsigned char length; + u16 vmin, vmax; + u16 target_rr; + u8 duration_incr_ms; + u8 duration_decr_ms; + u8 operation_mode; +}; + +void drm_dp_as_sdp_log(const char *level, struct device *dev, + const struct drm_dp_as_sdp *async); + void drm_dp_vsc_sdp_log(const char *level, struct device *dev, const struct drm_dp_vsc_sdp *vsc);
Add structure representing Adaptive Sync Secondary Data Packet (AS SDP). Also, add Adaptive Sync SDP logging in drm_dp_helper.c to facilitate debugging. Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> --- drivers/gpu/drm/display/drm_dp_helper.c | 15 +++++++++++++ include/drm/display/drm_dp.h | 1 + include/drm/display/drm_dp_helper.h | 30 +++++++++++++++++++++++++ 3 files changed, 46 insertions(+)