| Message ID | e9277a5635fc02106ca69b9071928c2249323500.1730740629.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/display: convert display feature helpers to struct intel_display | expand |
Hi Jani, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on next-20241105] [cannot apply to drm-xe/drm-xe-next linus/master drm-intel/for-linux-next-fixes drm-tip/drm-tip v6.12-rc6] [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/Jani-Nikula/drm-i915-display-identify-discrete-graphics/20241105-012305 base: git://anongit.freedesktop.org/drm-intel for-linux-next patch link: https://lore.kernel.org/r/e9277a5635fc02106ca69b9071928c2249323500.1730740629.git.jani.nikula%40intel.com patch subject: [PATCH 15/15] drm/i915/display: add mobile platform group config: x86_64-randconfig-123-20241105 (https://download.01.org/0day-ci/archive/20241106/202411060217.jLvmbe0Z-lkp@intel.com/config) compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241106/202411060217.jLvmbe0Z-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/202411060217.jLvmbe0Z-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/i915/display/intel_display_device.c:469:40: sparse: sparse: symbol 'snb_display' was not declared. Should it be static? drivers/gpu/drm/i915/display/intel_display_device.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/xarray.h, ...): include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false drivers/gpu/drm/i915/display/intel_display_device.c:555:25: sparse: sparse: Initializer entry defined twice drivers/gpu/drm/i915/display/intel_display_device.c:556:25: sparse: also defined here drivers/gpu/drm/i915/display/intel_display_device.c:608:25: sparse: sparse: Initializer entry defined twice drivers/gpu/drm/i915/display/intel_display_device.c:609:25: sparse: also defined here drivers/gpu/drm/i915/display/intel_display_device.c:857:17: sparse: sparse: Initializer entry defined twice drivers/gpu/drm/i915/display/intel_display_device.c:859:17: sparse: also defined here drivers/gpu/drm/i915/display/intel_display_device.c:1065:17: sparse: sparse: Initializer entry defined twice drivers/gpu/drm/i915/display/intel_display_device.c:1070:18: sparse: also defined here drivers/gpu/drm/i915/display/intel_display_device.c:1108:17: sparse: sparse: Initializer entry defined twice drivers/gpu/drm/i915/display/intel_display_device.c:1110:18: sparse: also defined here drivers/gpu/drm/i915/display/intel_display_device.c:1346:9: sparse: sparse: Initializer entry defined twice drivers/gpu/drm/i915/display/intel_display_device.c:1348:10: sparse: also defined here drivers/gpu/drm/i915/display/intel_display_device.c:1355:9: sparse: sparse: Initializer entry defined twice drivers/gpu/drm/i915/display/intel_display_device.c:1356:10: sparse: also defined here vim +/snb_display +469 drivers/gpu/drm/i915/display/intel_display_device.c 468 > 469 const struct intel_display_device_info snb_display = { 470 .has_hotplug = 1, 471 I9XX_PIPE_OFFSETS, 472 I9XX_CURSOR_OFFSETS, 473 ILK_COLORS, 474 475 .__runtime_defaults.ip.ver = 6, 476 .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), 477 .__runtime_defaults.cpu_transcoder_mask = 478 BIT(TRANSCODER_A) | BIT(TRANSCODER_B), 479 .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ 480 .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), 481 }; 482
On Mon, Nov 04, 2024 at 07:19:29PM +0200, Jani Nikula wrote: > Identify mobile platforms separately in display, using the platform > group mechanism. This enables dropping the dependency on i915_drv.h > IS_MOBILE() from display code. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > .../drm/i915/display/intel_display_device.c | 104 ++++++++++++------ > .../drm/i915/display/intel_display_device.h | 1 + > 2 files changed, 70 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c > index 8ba1b4652ca9..5689c5e5db0e 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_device.c > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c > @@ -253,6 +253,7 @@ static const struct intel_display_device_info no_display = {}; > > static const struct platform_desc i830_desc = { > PLATFORM(i830), > + PLATFORM_GROUP(mobile), > .info = &(const struct intel_display_device_info) { > I830_DISPLAY, > > @@ -271,6 +272,7 @@ static const struct platform_desc i845_desc = { > > static const struct platform_desc i85x_desc = { > PLATFORM(i85x), > + PLATFORM_GROUP(mobile), > .info = &(const struct intel_display_device_info) { > I830_DISPLAY, > > @@ -313,6 +315,7 @@ static const struct platform_desc i915g_desc = { > > static const struct platform_desc i915gm_desc = { > PLATFORM(i915gm), > + PLATFORM_GROUP(mobile), > .info = &(const struct intel_display_device_info) { > GEN3_DISPLAY, > I9XX_COLORS, > @@ -337,6 +340,7 @@ static const struct platform_desc i945g_desc = { > > static const struct platform_desc i945gm_desc = { > PLATFORM(i915gm), > + PLATFORM_GROUP(mobile), > .info = &(const struct intel_display_device_info) { > GEN3_DISPLAY, > I9XX_COLORS, > @@ -358,13 +362,21 @@ static const struct platform_desc g33_desc = { > }, > }; > > -static const struct platform_desc pnv_desc = { > +static const struct intel_display_device_info pnv_display = { > + GEN3_DISPLAY, > + I9XX_COLORS, > + .has_hotplug = 1, > +}; > + > +static const struct platform_desc pnv_g_desc = { > PLATFORM(pineview), > - .info = &(const struct intel_display_device_info) { > - GEN3_DISPLAY, > - I9XX_COLORS, > - .has_hotplug = 1, > - }, > + .info = &pnv_display, > +}; > + > +static const struct platform_desc pnv_m_desc = { > + PLATFORM(pineview), > + PLATFORM_GROUP(mobile), > + .info = &pnv_display, > }; > > #define GEN4_DISPLAY \ > @@ -391,6 +403,7 @@ static const struct platform_desc i965g_desc = { > > static const struct platform_desc i965gm_desc = { > PLATFORM(i965gm), > + PLATFORM_GROUP(mobile), > .info = &(const struct intel_display_device_info) { > GEN4_DISPLAY, > .has_overlay = 1, > @@ -414,6 +427,7 @@ static const struct platform_desc g45_desc = { > static const struct platform_desc gm45_desc = { > PLATFORM(gm45), > PLATFORM_GROUP(g4x), > + PLATFORM_GROUP(mobile), > .info = &(const struct intel_display_device_info) { > GEN4_DISPLAY, > .supports_tv = 1, > @@ -444,6 +458,7 @@ static const struct platform_desc ilk_d_desc = { > > static const struct platform_desc ilk_m_desc = { > PLATFORM(ironlake), > + PLATFORM_GROUP(mobile), > .info = &(const struct intel_display_device_info) { > ILK_DISPLAY, > > @@ -451,38 +466,54 @@ static const struct platform_desc ilk_m_desc = { > }, > }; > > -static const struct platform_desc snb_desc = { > +const struct intel_display_device_info snb_display = { get static back here as test robot already pointed out... > + .has_hotplug = 1, > + I9XX_PIPE_OFFSETS, > + I9XX_CURSOR_OFFSETS, > + ILK_COLORS, > + > + .__runtime_defaults.ip.ver = 6, > + .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), > + .__runtime_defaults.cpu_transcoder_mask = > + BIT(TRANSCODER_A) | BIT(TRANSCODER_B), > + .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ > + .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), > +}; > + > +static const struct platform_desc snb_d_desc = { > PLATFORM(sandybridge), > - .info = &(const struct intel_display_device_info) { > - .has_hotplug = 1, > - I9XX_PIPE_OFFSETS, > - I9XX_CURSOR_OFFSETS, > - ILK_COLORS, > + .info = &snb_display, > +}; > > - .__runtime_defaults.ip.ver = 6, > - .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), > - .__runtime_defaults.cpu_transcoder_mask = > - BIT(TRANSCODER_A) | BIT(TRANSCODER_B), > - .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ > - .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), > - }, > +static const struct platform_desc snb_m_desc = { > + PLATFORM(sandybridge), > + PLATFORM_GROUP(mobile), > + .info = &snb_display, > +}; > + > +static const struct intel_display_device_info ivb_display = { > + .has_hotplug = 1, > + IVB_PIPE_OFFSETS, > + IVB_CURSOR_OFFSETS, > + IVB_COLORS, > + > + .__runtime_defaults.ip.ver = 7, > + .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), > + .__runtime_defaults.cpu_transcoder_mask = > + BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C), > + .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ > + .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), > }; > > -static const struct platform_desc ivb_desc = { > +static const struct platform_desc ivb_d_desc = { > PLATFORM(ivybridge), > - .info = &(const struct intel_display_device_info) { > - .has_hotplug = 1, > - IVB_PIPE_OFFSETS, > - IVB_CURSOR_OFFSETS, > - IVB_COLORS, > + .info = &ivb_display, now I'm wondering 2 things: 1. is info really the good name now? 2. does it deserves a separate patch refactor other then this one focused on the mobile group? > +}; > > - .__runtime_defaults.ip.ver = 7, > - .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), > - .__runtime_defaults.cpu_transcoder_mask = > - BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C), > - .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ > - .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), > - }, > +static const struct platform_desc ivb_m_desc = { > + PLATFORM(ivybridge), > + PLATFORM_GROUP(mobile), > + .info = &ivb_display, > }; > > static const struct platform_desc vlv_desc = { > @@ -1385,11 +1416,14 @@ static const struct { > INTEL_I965GM_IDS(INTEL_DISPLAY_DEVICE, &i965gm_desc), > INTEL_GM45_IDS(INTEL_DISPLAY_DEVICE, &gm45_desc), > INTEL_G45_IDS(INTEL_DISPLAY_DEVICE, &g45_desc), > - INTEL_PNV_IDS(INTEL_DISPLAY_DEVICE, &pnv_desc), > + INTEL_PNV_G_IDS(INTEL_DISPLAY_DEVICE, &pnv_g_desc), > + INTEL_PNV_M_IDS(INTEL_DISPLAY_DEVICE, &pnv_m_desc), > INTEL_ILK_D_IDS(INTEL_DISPLAY_DEVICE, &ilk_d_desc), > INTEL_ILK_M_IDS(INTEL_DISPLAY_DEVICE, &ilk_m_desc), > - INTEL_SNB_IDS(INTEL_DISPLAY_DEVICE, &snb_desc), > - INTEL_IVB_IDS(INTEL_DISPLAY_DEVICE, &ivb_desc), > + INTEL_SNB_D_IDS(INTEL_DISPLAY_DEVICE, &snb_d_desc), > + INTEL_SNB_M_IDS(INTEL_DISPLAY_DEVICE, &snb_m_desc), > + INTEL_IVB_D_IDS(INTEL_DISPLAY_DEVICE, &ivb_d_desc), > + INTEL_IVB_M_IDS(INTEL_DISPLAY_DEVICE, &ivb_m_desc), > INTEL_HSW_IDS(INTEL_DISPLAY_DEVICE, &hsw_desc), > INTEL_VLV_IDS(INTEL_DISPLAY_DEVICE, &vlv_desc), > INTEL_BDW_IDS(INTEL_DISPLAY_DEVICE, &bdw_desc), > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h > index e1398689cda5..84378c787923 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_device.h > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h > @@ -24,6 +24,7 @@ struct pci_dev; > #define INTEL_DISPLAY_PLATFORMS(func) \ > /* Platform group aliases */ \ > func(g4x) /* g45 and gm45 */ \ > + func(mobile) /* mobile platforms */ \ > func(dgfx) /* discrete graphics */ \ > /* Display ver 2 */ \ > func(i830) \ > -- > 2.39.5 >
On Wed, 06 Nov 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > On Mon, Nov 04, 2024 at 07:19:29PM +0200, Jani Nikula wrote: >> Identify mobile platforms separately in display, using the platform >> group mechanism. This enables dropping the dependency on i915_drv.h >> IS_MOBILE() from display code. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> .../drm/i915/display/intel_display_device.c | 104 ++++++++++++------ >> .../drm/i915/display/intel_display_device.h | 1 + >> 2 files changed, 70 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c >> index 8ba1b4652ca9..5689c5e5db0e 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_device.c >> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c >> @@ -253,6 +253,7 @@ static const struct intel_display_device_info no_display = {}; >> >> static const struct platform_desc i830_desc = { >> PLATFORM(i830), >> + PLATFORM_GROUP(mobile), >> .info = &(const struct intel_display_device_info) { >> I830_DISPLAY, >> >> @@ -271,6 +272,7 @@ static const struct platform_desc i845_desc = { >> >> static const struct platform_desc i85x_desc = { >> PLATFORM(i85x), >> + PLATFORM_GROUP(mobile), >> .info = &(const struct intel_display_device_info) { >> I830_DISPLAY, >> >> @@ -313,6 +315,7 @@ static const struct platform_desc i915g_desc = { >> >> static const struct platform_desc i915gm_desc = { >> PLATFORM(i915gm), >> + PLATFORM_GROUP(mobile), >> .info = &(const struct intel_display_device_info) { >> GEN3_DISPLAY, >> I9XX_COLORS, >> @@ -337,6 +340,7 @@ static const struct platform_desc i945g_desc = { >> >> static const struct platform_desc i945gm_desc = { >> PLATFORM(i915gm), >> + PLATFORM_GROUP(mobile), >> .info = &(const struct intel_display_device_info) { >> GEN3_DISPLAY, >> I9XX_COLORS, >> @@ -358,13 +362,21 @@ static const struct platform_desc g33_desc = { >> }, >> }; >> >> -static const struct platform_desc pnv_desc = { >> +static const struct intel_display_device_info pnv_display = { >> + GEN3_DISPLAY, >> + I9XX_COLORS, >> + .has_hotplug = 1, >> +}; >> + >> +static const struct platform_desc pnv_g_desc = { >> PLATFORM(pineview), >> - .info = &(const struct intel_display_device_info) { >> - GEN3_DISPLAY, >> - I9XX_COLORS, >> - .has_hotplug = 1, >> - }, >> + .info = &pnv_display, >> +}; >> + >> +static const struct platform_desc pnv_m_desc = { >> + PLATFORM(pineview), >> + PLATFORM_GROUP(mobile), >> + .info = &pnv_display, >> }; >> >> #define GEN4_DISPLAY \ >> @@ -391,6 +403,7 @@ static const struct platform_desc i965g_desc = { >> >> static const struct platform_desc i965gm_desc = { >> PLATFORM(i965gm), >> + PLATFORM_GROUP(mobile), >> .info = &(const struct intel_display_device_info) { >> GEN4_DISPLAY, >> .has_overlay = 1, >> @@ -414,6 +427,7 @@ static const struct platform_desc g45_desc = { >> static const struct platform_desc gm45_desc = { >> PLATFORM(gm45), >> PLATFORM_GROUP(g4x), >> + PLATFORM_GROUP(mobile), >> .info = &(const struct intel_display_device_info) { >> GEN4_DISPLAY, >> .supports_tv = 1, >> @@ -444,6 +458,7 @@ static const struct platform_desc ilk_d_desc = { >> >> static const struct platform_desc ilk_m_desc = { >> PLATFORM(ironlake), >> + PLATFORM_GROUP(mobile), >> .info = &(const struct intel_display_device_info) { >> ILK_DISPLAY, >> >> @@ -451,38 +466,54 @@ static const struct platform_desc ilk_m_desc = { >> }, >> }; >> >> -static const struct platform_desc snb_desc = { >> +const struct intel_display_device_info snb_display = { > > get static back here as test robot already pointed out... > >> + .has_hotplug = 1, >> + I9XX_PIPE_OFFSETS, >> + I9XX_CURSOR_OFFSETS, >> + ILK_COLORS, >> + >> + .__runtime_defaults.ip.ver = 6, >> + .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), >> + .__runtime_defaults.cpu_transcoder_mask = >> + BIT(TRANSCODER_A) | BIT(TRANSCODER_B), >> + .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ >> + .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), >> +}; >> + >> +static const struct platform_desc snb_d_desc = { >> PLATFORM(sandybridge), >> - .info = &(const struct intel_display_device_info) { >> - .has_hotplug = 1, >> - I9XX_PIPE_OFFSETS, >> - I9XX_CURSOR_OFFSETS, >> - ILK_COLORS, >> + .info = &snb_display, >> +}; >> >> - .__runtime_defaults.ip.ver = 6, >> - .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), >> - .__runtime_defaults.cpu_transcoder_mask = >> - BIT(TRANSCODER_A) | BIT(TRANSCODER_B), >> - .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ >> - .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), >> - }, >> +static const struct platform_desc snb_m_desc = { >> + PLATFORM(sandybridge), >> + PLATFORM_GROUP(mobile), >> + .info = &snb_display, >> +}; >> + >> +static const struct intel_display_device_info ivb_display = { >> + .has_hotplug = 1, >> + IVB_PIPE_OFFSETS, >> + IVB_CURSOR_OFFSETS, >> + IVB_COLORS, >> + >> + .__runtime_defaults.ip.ver = 7, >> + .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), >> + .__runtime_defaults.cpu_transcoder_mask = >> + BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C), >> + .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ >> + .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), >> }; >> >> -static const struct platform_desc ivb_desc = { >> +static const struct platform_desc ivb_d_desc = { >> PLATFORM(ivybridge), >> - .info = &(const struct intel_display_device_info) { >> - .has_hotplug = 1, >> - IVB_PIPE_OFFSETS, >> - IVB_CURSOR_OFFSETS, >> - IVB_COLORS, >> + .info = &ivb_display, > > now I'm wondering 2 things: > > 1. is info really the good name now? Why not? Desc is the platform description, unrelated to display, and info is the display info. In some cases display info was (and still is) inline, but when it's shared between multiple platforms it needs to be separate. > 2. does it deserves a separate patch refactor other then this one focused > on the mobile group? Un-inlining the dispay info init could reasonably be a separate patch. Question is, do we then want to do it for all platforms, or just the ones that need the split here? BR, Jani. > >> +}; >> >> - .__runtime_defaults.ip.ver = 7, >> - .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), >> - .__runtime_defaults.cpu_transcoder_mask = >> - BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C), >> - .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ >> - .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), >> - }, >> +static const struct platform_desc ivb_m_desc = { >> + PLATFORM(ivybridge), >> + PLATFORM_GROUP(mobile), >> + .info = &ivb_display, >> }; >> >> static const struct platform_desc vlv_desc = { >> @@ -1385,11 +1416,14 @@ static const struct { >> INTEL_I965GM_IDS(INTEL_DISPLAY_DEVICE, &i965gm_desc), >> INTEL_GM45_IDS(INTEL_DISPLAY_DEVICE, &gm45_desc), >> INTEL_G45_IDS(INTEL_DISPLAY_DEVICE, &g45_desc), >> - INTEL_PNV_IDS(INTEL_DISPLAY_DEVICE, &pnv_desc), >> + INTEL_PNV_G_IDS(INTEL_DISPLAY_DEVICE, &pnv_g_desc), >> + INTEL_PNV_M_IDS(INTEL_DISPLAY_DEVICE, &pnv_m_desc), >> INTEL_ILK_D_IDS(INTEL_DISPLAY_DEVICE, &ilk_d_desc), >> INTEL_ILK_M_IDS(INTEL_DISPLAY_DEVICE, &ilk_m_desc), >> - INTEL_SNB_IDS(INTEL_DISPLAY_DEVICE, &snb_desc), >> - INTEL_IVB_IDS(INTEL_DISPLAY_DEVICE, &ivb_desc), >> + INTEL_SNB_D_IDS(INTEL_DISPLAY_DEVICE, &snb_d_desc), >> + INTEL_SNB_M_IDS(INTEL_DISPLAY_DEVICE, &snb_m_desc), >> + INTEL_IVB_D_IDS(INTEL_DISPLAY_DEVICE, &ivb_d_desc), >> + INTEL_IVB_M_IDS(INTEL_DISPLAY_DEVICE, &ivb_m_desc), >> INTEL_HSW_IDS(INTEL_DISPLAY_DEVICE, &hsw_desc), >> INTEL_VLV_IDS(INTEL_DISPLAY_DEVICE, &vlv_desc), >> INTEL_BDW_IDS(INTEL_DISPLAY_DEVICE, &bdw_desc), >> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h >> index e1398689cda5..84378c787923 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_device.h >> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h >> @@ -24,6 +24,7 @@ struct pci_dev; >> #define INTEL_DISPLAY_PLATFORMS(func) \ >> /* Platform group aliases */ \ >> func(g4x) /* g45 and gm45 */ \ >> + func(mobile) /* mobile platforms */ \ >> func(dgfx) /* discrete graphics */ \ >> /* Display ver 2 */ \ >> func(i830) \ >> -- >> 2.39.5 >>
On Thu, Nov 07, 2024 at 12:32:56PM +0200, Jani Nikula wrote: > On Wed, 06 Nov 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > > On Mon, Nov 04, 2024 at 07:19:29PM +0200, Jani Nikula wrote: > >> Identify mobile platforms separately in display, using the platform > >> group mechanism. This enables dropping the dependency on i915_drv.h > >> IS_MOBILE() from display code. > >> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> --- > >> .../drm/i915/display/intel_display_device.c | 104 ++++++++++++------ > >> .../drm/i915/display/intel_display_device.h | 1 + > >> 2 files changed, 70 insertions(+), 35 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c > >> index 8ba1b4652ca9..5689c5e5db0e 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display_device.c > >> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c > >> @@ -253,6 +253,7 @@ static const struct intel_display_device_info no_display = {}; > >> > >> static const struct platform_desc i830_desc = { > >> PLATFORM(i830), > >> + PLATFORM_GROUP(mobile), > >> .info = &(const struct intel_display_device_info) { > >> I830_DISPLAY, > >> > >> @@ -271,6 +272,7 @@ static const struct platform_desc i845_desc = { > >> > >> static const struct platform_desc i85x_desc = { > >> PLATFORM(i85x), > >> + PLATFORM_GROUP(mobile), > >> .info = &(const struct intel_display_device_info) { > >> I830_DISPLAY, > >> > >> @@ -313,6 +315,7 @@ static const struct platform_desc i915g_desc = { > >> > >> static const struct platform_desc i915gm_desc = { > >> PLATFORM(i915gm), > >> + PLATFORM_GROUP(mobile), > >> .info = &(const struct intel_display_device_info) { > >> GEN3_DISPLAY, > >> I9XX_COLORS, > >> @@ -337,6 +340,7 @@ static const struct platform_desc i945g_desc = { > >> > >> static const struct platform_desc i945gm_desc = { > >> PLATFORM(i915gm), > >> + PLATFORM_GROUP(mobile), > >> .info = &(const struct intel_display_device_info) { > >> GEN3_DISPLAY, > >> I9XX_COLORS, > >> @@ -358,13 +362,21 @@ static const struct platform_desc g33_desc = { > >> }, > >> }; > >> > >> -static const struct platform_desc pnv_desc = { > >> +static const struct intel_display_device_info pnv_display = { > >> + GEN3_DISPLAY, > >> + I9XX_COLORS, > >> + .has_hotplug = 1, > >> +}; > >> + > >> +static const struct platform_desc pnv_g_desc = { > >> PLATFORM(pineview), > >> - .info = &(const struct intel_display_device_info) { > >> - GEN3_DISPLAY, > >> - I9XX_COLORS, > >> - .has_hotplug = 1, > >> - }, > >> + .info = &pnv_display, > >> +}; > >> + > >> +static const struct platform_desc pnv_m_desc = { > >> + PLATFORM(pineview), > >> + PLATFORM_GROUP(mobile), > >> + .info = &pnv_display, > >> }; > >> > >> #define GEN4_DISPLAY \ > >> @@ -391,6 +403,7 @@ static const struct platform_desc i965g_desc = { > >> > >> static const struct platform_desc i965gm_desc = { > >> PLATFORM(i965gm), > >> + PLATFORM_GROUP(mobile), > >> .info = &(const struct intel_display_device_info) { > >> GEN4_DISPLAY, > >> .has_overlay = 1, > >> @@ -414,6 +427,7 @@ static const struct platform_desc g45_desc = { > >> static const struct platform_desc gm45_desc = { > >> PLATFORM(gm45), > >> PLATFORM_GROUP(g4x), > >> + PLATFORM_GROUP(mobile), > >> .info = &(const struct intel_display_device_info) { > >> GEN4_DISPLAY, > >> .supports_tv = 1, > >> @@ -444,6 +458,7 @@ static const struct platform_desc ilk_d_desc = { > >> > >> static const struct platform_desc ilk_m_desc = { > >> PLATFORM(ironlake), > >> + PLATFORM_GROUP(mobile), > >> .info = &(const struct intel_display_device_info) { > >> ILK_DISPLAY, > >> > >> @@ -451,38 +466,54 @@ static const struct platform_desc ilk_m_desc = { > >> }, > >> }; > >> > >> -static const struct platform_desc snb_desc = { > >> +const struct intel_display_device_info snb_display = { > > > > get static back here as test robot already pointed out... > > > >> + .has_hotplug = 1, > >> + I9XX_PIPE_OFFSETS, > >> + I9XX_CURSOR_OFFSETS, > >> + ILK_COLORS, > >> + > >> + .__runtime_defaults.ip.ver = 6, > >> + .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), > >> + .__runtime_defaults.cpu_transcoder_mask = > >> + BIT(TRANSCODER_A) | BIT(TRANSCODER_B), > >> + .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ > >> + .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), > >> +}; > >> + > >> +static const struct platform_desc snb_d_desc = { > >> PLATFORM(sandybridge), > >> - .info = &(const struct intel_display_device_info) { > >> - .has_hotplug = 1, > >> - I9XX_PIPE_OFFSETS, > >> - I9XX_CURSOR_OFFSETS, > >> - ILK_COLORS, > >> + .info = &snb_display, > >> +}; > >> > >> - .__runtime_defaults.ip.ver = 6, > >> - .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), > >> - .__runtime_defaults.cpu_transcoder_mask = > >> - BIT(TRANSCODER_A) | BIT(TRANSCODER_B), > >> - .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ > >> - .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), > >> - }, > >> +static const struct platform_desc snb_m_desc = { > >> + PLATFORM(sandybridge), > >> + PLATFORM_GROUP(mobile), > >> + .info = &snb_display, > >> +}; > >> + > >> +static const struct intel_display_device_info ivb_display = { > >> + .has_hotplug = 1, > >> + IVB_PIPE_OFFSETS, > >> + IVB_CURSOR_OFFSETS, > >> + IVB_COLORS, > >> + > >> + .__runtime_defaults.ip.ver = 7, > >> + .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), > >> + .__runtime_defaults.cpu_transcoder_mask = > >> + BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C), > >> + .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ > >> + .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), > >> }; > >> > >> -static const struct platform_desc ivb_desc = { > >> +static const struct platform_desc ivb_d_desc = { > >> PLATFORM(ivybridge), > >> - .info = &(const struct intel_display_device_info) { > >> - .has_hotplug = 1, > >> - IVB_PIPE_OFFSETS, > >> - IVB_CURSOR_OFFSETS, > >> - IVB_COLORS, > >> + .info = &ivb_display, > > > > now I'm wondering 2 things: > > > > 1. is info really the good name now? > > Why not? > > Desc is the platform description, unrelated to display, and info is the > display info. In some cases display info was (and still is) inline, but > when it's shared between multiple platforms it needs to be separate. > > > 2. does it deserves a separate patch refactor other then this one focused > > on the mobile group? > > Un-inlining the dispay info init could reasonably be a separate > patch. Question is, do we then want to do it for all platforms, or just > the ones that need the split here? okay, let's just go as is Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > BR, > Jani. > > > > > >> +}; > >> > >> - .__runtime_defaults.ip.ver = 7, > >> - .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), > >> - .__runtime_defaults.cpu_transcoder_mask = > >> - BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C), > >> - .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ > >> - .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), > >> - }, > >> +static const struct platform_desc ivb_m_desc = { > >> + PLATFORM(ivybridge), > >> + PLATFORM_GROUP(mobile), > >> + .info = &ivb_display, > >> }; > >> > >> static const struct platform_desc vlv_desc = { > >> @@ -1385,11 +1416,14 @@ static const struct { > >> INTEL_I965GM_IDS(INTEL_DISPLAY_DEVICE, &i965gm_desc), > >> INTEL_GM45_IDS(INTEL_DISPLAY_DEVICE, &gm45_desc), > >> INTEL_G45_IDS(INTEL_DISPLAY_DEVICE, &g45_desc), > >> - INTEL_PNV_IDS(INTEL_DISPLAY_DEVICE, &pnv_desc), > >> + INTEL_PNV_G_IDS(INTEL_DISPLAY_DEVICE, &pnv_g_desc), > >> + INTEL_PNV_M_IDS(INTEL_DISPLAY_DEVICE, &pnv_m_desc), > >> INTEL_ILK_D_IDS(INTEL_DISPLAY_DEVICE, &ilk_d_desc), > >> INTEL_ILK_M_IDS(INTEL_DISPLAY_DEVICE, &ilk_m_desc), > >> - INTEL_SNB_IDS(INTEL_DISPLAY_DEVICE, &snb_desc), > >> - INTEL_IVB_IDS(INTEL_DISPLAY_DEVICE, &ivb_desc), > >> + INTEL_SNB_D_IDS(INTEL_DISPLAY_DEVICE, &snb_d_desc), > >> + INTEL_SNB_M_IDS(INTEL_DISPLAY_DEVICE, &snb_m_desc), > >> + INTEL_IVB_D_IDS(INTEL_DISPLAY_DEVICE, &ivb_d_desc), > >> + INTEL_IVB_M_IDS(INTEL_DISPLAY_DEVICE, &ivb_m_desc), > >> INTEL_HSW_IDS(INTEL_DISPLAY_DEVICE, &hsw_desc), > >> INTEL_VLV_IDS(INTEL_DISPLAY_DEVICE, &vlv_desc), > >> INTEL_BDW_IDS(INTEL_DISPLAY_DEVICE, &bdw_desc), > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h > >> index e1398689cda5..84378c787923 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display_device.h > >> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h > >> @@ -24,6 +24,7 @@ struct pci_dev; > >> #define INTEL_DISPLAY_PLATFORMS(func) \ > >> /* Platform group aliases */ \ > >> func(g4x) /* g45 and gm45 */ \ > >> + func(mobile) /* mobile platforms */ \ > >> func(dgfx) /* discrete graphics */ \ > >> /* Display ver 2 */ \ > >> func(i830) \ > >> -- > >> 2.39.5 > >> > > -- > Jani Nikula, Intel
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c index 8ba1b4652ca9..5689c5e5db0e 100644 --- a/drivers/gpu/drm/i915/display/intel_display_device.c +++ b/drivers/gpu/drm/i915/display/intel_display_device.c @@ -253,6 +253,7 @@ static const struct intel_display_device_info no_display = {}; static const struct platform_desc i830_desc = { PLATFORM(i830), + PLATFORM_GROUP(mobile), .info = &(const struct intel_display_device_info) { I830_DISPLAY, @@ -271,6 +272,7 @@ static const struct platform_desc i845_desc = { static const struct platform_desc i85x_desc = { PLATFORM(i85x), + PLATFORM_GROUP(mobile), .info = &(const struct intel_display_device_info) { I830_DISPLAY, @@ -313,6 +315,7 @@ static const struct platform_desc i915g_desc = { static const struct platform_desc i915gm_desc = { PLATFORM(i915gm), + PLATFORM_GROUP(mobile), .info = &(const struct intel_display_device_info) { GEN3_DISPLAY, I9XX_COLORS, @@ -337,6 +340,7 @@ static const struct platform_desc i945g_desc = { static const struct platform_desc i945gm_desc = { PLATFORM(i915gm), + PLATFORM_GROUP(mobile), .info = &(const struct intel_display_device_info) { GEN3_DISPLAY, I9XX_COLORS, @@ -358,13 +362,21 @@ static const struct platform_desc g33_desc = { }, }; -static const struct platform_desc pnv_desc = { +static const struct intel_display_device_info pnv_display = { + GEN3_DISPLAY, + I9XX_COLORS, + .has_hotplug = 1, +}; + +static const struct platform_desc pnv_g_desc = { PLATFORM(pineview), - .info = &(const struct intel_display_device_info) { - GEN3_DISPLAY, - I9XX_COLORS, - .has_hotplug = 1, - }, + .info = &pnv_display, +}; + +static const struct platform_desc pnv_m_desc = { + PLATFORM(pineview), + PLATFORM_GROUP(mobile), + .info = &pnv_display, }; #define GEN4_DISPLAY \ @@ -391,6 +403,7 @@ static const struct platform_desc i965g_desc = { static const struct platform_desc i965gm_desc = { PLATFORM(i965gm), + PLATFORM_GROUP(mobile), .info = &(const struct intel_display_device_info) { GEN4_DISPLAY, .has_overlay = 1, @@ -414,6 +427,7 @@ static const struct platform_desc g45_desc = { static const struct platform_desc gm45_desc = { PLATFORM(gm45), PLATFORM_GROUP(g4x), + PLATFORM_GROUP(mobile), .info = &(const struct intel_display_device_info) { GEN4_DISPLAY, .supports_tv = 1, @@ -444,6 +458,7 @@ static const struct platform_desc ilk_d_desc = { static const struct platform_desc ilk_m_desc = { PLATFORM(ironlake), + PLATFORM_GROUP(mobile), .info = &(const struct intel_display_device_info) { ILK_DISPLAY, @@ -451,38 +466,54 @@ static const struct platform_desc ilk_m_desc = { }, }; -static const struct platform_desc snb_desc = { +const struct intel_display_device_info snb_display = { + .has_hotplug = 1, + I9XX_PIPE_OFFSETS, + I9XX_CURSOR_OFFSETS, + ILK_COLORS, + + .__runtime_defaults.ip.ver = 6, + .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), + .__runtime_defaults.cpu_transcoder_mask = + BIT(TRANSCODER_A) | BIT(TRANSCODER_B), + .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ + .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), +}; + +static const struct platform_desc snb_d_desc = { PLATFORM(sandybridge), - .info = &(const struct intel_display_device_info) { - .has_hotplug = 1, - I9XX_PIPE_OFFSETS, - I9XX_CURSOR_OFFSETS, - ILK_COLORS, + .info = &snb_display, +}; - .__runtime_defaults.ip.ver = 6, - .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), - .__runtime_defaults.cpu_transcoder_mask = - BIT(TRANSCODER_A) | BIT(TRANSCODER_B), - .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ - .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), - }, +static const struct platform_desc snb_m_desc = { + PLATFORM(sandybridge), + PLATFORM_GROUP(mobile), + .info = &snb_display, +}; + +static const struct intel_display_device_info ivb_display = { + .has_hotplug = 1, + IVB_PIPE_OFFSETS, + IVB_CURSOR_OFFSETS, + IVB_COLORS, + + .__runtime_defaults.ip.ver = 7, + .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), + .__runtime_defaults.cpu_transcoder_mask = + BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C), + .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ + .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), }; -static const struct platform_desc ivb_desc = { +static const struct platform_desc ivb_d_desc = { PLATFORM(ivybridge), - .info = &(const struct intel_display_device_info) { - .has_hotplug = 1, - IVB_PIPE_OFFSETS, - IVB_CURSOR_OFFSETS, - IVB_COLORS, + .info = &ivb_display, +}; - .__runtime_defaults.ip.ver = 7, - .__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), - .__runtime_defaults.cpu_transcoder_mask = - BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C), - .__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */ - .__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A), - }, +static const struct platform_desc ivb_m_desc = { + PLATFORM(ivybridge), + PLATFORM_GROUP(mobile), + .info = &ivb_display, }; static const struct platform_desc vlv_desc = { @@ -1385,11 +1416,14 @@ static const struct { INTEL_I965GM_IDS(INTEL_DISPLAY_DEVICE, &i965gm_desc), INTEL_GM45_IDS(INTEL_DISPLAY_DEVICE, &gm45_desc), INTEL_G45_IDS(INTEL_DISPLAY_DEVICE, &g45_desc), - INTEL_PNV_IDS(INTEL_DISPLAY_DEVICE, &pnv_desc), + INTEL_PNV_G_IDS(INTEL_DISPLAY_DEVICE, &pnv_g_desc), + INTEL_PNV_M_IDS(INTEL_DISPLAY_DEVICE, &pnv_m_desc), INTEL_ILK_D_IDS(INTEL_DISPLAY_DEVICE, &ilk_d_desc), INTEL_ILK_M_IDS(INTEL_DISPLAY_DEVICE, &ilk_m_desc), - INTEL_SNB_IDS(INTEL_DISPLAY_DEVICE, &snb_desc), - INTEL_IVB_IDS(INTEL_DISPLAY_DEVICE, &ivb_desc), + INTEL_SNB_D_IDS(INTEL_DISPLAY_DEVICE, &snb_d_desc), + INTEL_SNB_M_IDS(INTEL_DISPLAY_DEVICE, &snb_m_desc), + INTEL_IVB_D_IDS(INTEL_DISPLAY_DEVICE, &ivb_d_desc), + INTEL_IVB_M_IDS(INTEL_DISPLAY_DEVICE, &ivb_m_desc), INTEL_HSW_IDS(INTEL_DISPLAY_DEVICE, &hsw_desc), INTEL_VLV_IDS(INTEL_DISPLAY_DEVICE, &vlv_desc), INTEL_BDW_IDS(INTEL_DISPLAY_DEVICE, &bdw_desc), diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h index e1398689cda5..84378c787923 100644 --- a/drivers/gpu/drm/i915/display/intel_display_device.h +++ b/drivers/gpu/drm/i915/display/intel_display_device.h @@ -24,6 +24,7 @@ struct pci_dev; #define INTEL_DISPLAY_PLATFORMS(func) \ /* Platform group aliases */ \ func(g4x) /* g45 and gm45 */ \ + func(mobile) /* mobile platforms */ \ func(dgfx) /* discrete graphics */ \ /* Display ver 2 */ \ func(i830) \
Identify mobile platforms separately in display, using the platform group mechanism. This enables dropping the dependency on i915_drv.h IS_MOBILE() from display code. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- .../drm/i915/display/intel_display_device.c | 104 ++++++++++++------ .../drm/i915/display/intel_display_device.h | 1 + 2 files changed, 70 insertions(+), 35 deletions(-)