RE: [PATCH 3/3] drm/xe: Update GuC/HuC firmware autoselect logic
> -Original Message- > From: De Marchi, Lucas > Sent: Wednesday, March 29, 2023 8:46 PM > To: Srivatsa, Anusha > Cc: intel...@lists.freedesktop.org; Harrison, John C > ; Ceraolo Spurio, Daniele > ; dri-devel@lists.freedesktop.org; Daniel > Vetter ; Dave Airlie > Subject: Re: [PATCH 3/3] drm/xe: Update GuC/HuC firmware autoselect logic > > On Tue, Mar 28, 2023 at 04:31:13PM -0700, Anusha Srivatsa wrote: > > > > > >> -Original Message- > >> From: De Marchi, Lucas > >> Sent: Thursday, March 23, 2023 10:18 PM > >> To: intel...@lists.freedesktop.org > >> Cc: Srivatsa, Anusha ; Harrison, John C > >> ; Ceraolo Spurio, Daniele > >> ; dri-devel@lists.freedesktop.org; > >> Daniel Vetter ; Dave Airlie > >> ; De Marchi, Lucas > >> Subject: [PATCH 3/3] drm/xe: Update GuC/HuC firmware autoselect logic > >> > >> Update the logic to autoselect GuC/HuC for the platforms with the > >> following > >> improvements: > >> > >> - Document what is the firmware file that is expected to be > >> loaded and what is checked from blob headers > >> - When the platform is under force-probe it's desired to enforce > >> the full-version requirement so the correct firmware is used > >> before widespread adoption and backward-compatibility > >> > >Extra line ^ > > > >> commitments > >> - Directory from which we expect firmware blobs to be available in > >> upstream linux-firmware repository depends on the platform: for > >> the ones supported by i915 it uses the i915/ directory, but the ones > >> expected to be supported by xe, it's on the xe/ directory. This > >> means that for platforms in the intersection, the firmware is > >> loaded from a different directory, but that is not much important > >> in the firmware repo and it avoids firmware duplication. > >> > >> - Make the table with the firmware definitions clearly state the > >> versions being expected. Now with macros to select the version it's > >> possible to choose between full-version/major-version for GuC and > >> full-version/no-version for HuC. These are similar to the macros used > >> in i915, but implemented in a slightly different way to avoid > >> duplicating the macros for each firmware/type and functionality, > >> besides adding the support for different directories. > >> > >> - There is no check added regarding force-probe since xe should > >> reuse the same firmware files published for i915 for past > >> platforms. This can be improved later with additional > >> kunit checking against a hardcoded list of platforms that > >Extra line here. > > > >> falls in this category. > >> - As mentioned in the TODO, the major version fallback was not > >> implemented before as currently each platform only supports one > >> major. That can be easily added later. > >> > >> - GuC version for MTL and PVC were updated to 70.6.4, using the exact > >> full version, while the > >> > >> After this the GuC firmware used by PVC changes to pvc_guc_70.5.2.bin > >> since it's using a file not published yet. > >> > >> Signed-off-by: Lucas De Marchi > >> --- > >> drivers/gpu/drm/xe/xe_uc_fw.c | 315 +--- > >> drivers/gpu/drm/xe/xe_uc_fw.h | 2 +- > >> drivers/gpu/drm/xe/xe_uc_fw_types.h | 7 + > >> 3 files changed, 204 insertions(+), 120 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c > >> b/drivers/gpu/drm/xe/xe_uc_fw.c index 174c42873ebb..653bc3584cc5 > >> 100644 > >> --- a/drivers/gpu/drm/xe/xe_uc_fw.c > >> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c > >> @@ -17,6 +17,137 @@ > >> #include "xe_mmio.h" > >> #include "xe_uc_fw.h" > >> > >> +/* > >> + * List of required GuC and HuC binaries per-platform. They must be > >> +ordered > >> + * based on platform, from newer to older. > >> + * > >> + * Versioning follows the guidelines from > >> + * Documentation/driver-api/firmware/firmware-usage-guidelines.rst. > >> +There is a > >> + * distinction for platforms being officially supported by the driver or > >> not. > >> + * Platforms not available publicly or not yet officially supported > >> +by the > >> + * driver (under force-probe), use the mmp_ver(): the firmw
RE: [PATCH 3/3] drm/xe: Update GuC/HuC firmware autoselect logic
> -Original Message- > From: De Marchi, Lucas > Sent: Thursday, March 23, 2023 10:18 PM > To: intel...@lists.freedesktop.org > Cc: Srivatsa, Anusha ; Harrison, John C > ; Ceraolo Spurio, Daniele > ; dri-devel@lists.freedesktop.org; Daniel > Vetter ; Dave Airlie ; De Marchi, > Lucas > Subject: [PATCH 3/3] drm/xe: Update GuC/HuC firmware autoselect logic > > Update the logic to autoselect GuC/HuC for the platforms with the following > improvements: > > - Document what is the firmware file that is expected to be > loaded and what is checked from blob headers > - When the platform is under force-probe it's desired to enforce > the full-version requirement so the correct firmware is used > before widespread adoption and backward-compatibility > Extra line ^ > commitments > - Directory from which we expect firmware blobs to be available in > upstream linux-firmware repository depends on the platform: for > the ones supported by i915 it uses the i915/ directory, but the ones > expected to be supported by xe, it's on the xe/ directory. This > means that for platforms in the intersection, the firmware is > loaded from a different directory, but that is not much important > in the firmware repo and it avoids firmware duplication. > > - Make the table with the firmware definitions clearly state the > versions being expected. Now with macros to select the version it's > possible to choose between full-version/major-version for GuC and > full-version/no-version for HuC. These are similar to the macros used > in i915, but implemented in a slightly different way to avoid > duplicating the macros for each firmware/type and functionality, > besides adding the support for different directories. > > - There is no check added regarding force-probe since xe should > reuse the same firmware files published for i915 for past > platforms. This can be improved later with additional > kunit checking against a hardcoded list of platforms that Extra line here. > falls in this category. > - As mentioned in the TODO, the major version fallback was not > implemented before as currently each platform only supports one > major. That can be easily added later. > > - GuC version for MTL and PVC were updated to 70.6.4, using the exact > full version, while the > > After this the GuC firmware used by PVC changes to pvc_guc_70.5.2.bin since > it's > using a file not published yet. > > Signed-off-by: Lucas De Marchi > --- > drivers/gpu/drm/xe/xe_uc_fw.c | 315 +--- > drivers/gpu/drm/xe/xe_uc_fw.h | 2 +- > drivers/gpu/drm/xe/xe_uc_fw_types.h | 7 + > 3 files changed, 204 insertions(+), 120 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c > index 174c42873ebb..653bc3584cc5 100644 > --- a/drivers/gpu/drm/xe/xe_uc_fw.c > +++ b/drivers/gpu/drm/xe/xe_uc_fw.c > @@ -17,6 +17,137 @@ > #include "xe_mmio.h" > #include "xe_uc_fw.h" > > +/* > + * List of required GuC and HuC binaries per-platform. They must be > +ordered > + * based on platform, from newer to older. > + * > + * Versioning follows the guidelines from > + * Documentation/driver-api/firmware/firmware-usage-guidelines.rst. > +There is a > + * distinction for platforms being officially supported by the driver or not. > + * Platforms not available publicly or not yet officially supported by > +the > + * driver (under force-probe), use the mmp_ver(): the firmware > +autoselect logic > + * will select the firmware from disk with filename that matches the > +full > + * "mpp version", i.e. major.minor.patch. mmp_ver() should only be used > +for > + * this case. > + * > + * For platforms officially supported by the driver, the filename > +always only > + * ever contains the major version (GuC) or no version at all (HuC). > + * > + * After loading the file, the driver parses the versions embedded in the > blob. > + * The major version needs to match a major version supported by the > +driver (if > + * any). The minor version is also checked and a notice emitted to the > +log if > + * the version found is smaller than the version wanted. This is done > +only for > + * informational purposes so users may have a chance to upgrade, but > +the driver > + * still loads and use the older firmware. > + * > + * Examples: > + * > + * 1) Platform officially supported by i915 - using Tigerlake as example. > + * Driver loads the following firmware blobs from disk: > + * > + * - i915/tgl_guc_.bin > + * - i915/tgl_huc.bin > + * > + * number for GuC is checked tha
RE: [Intel-gfx] [PATCH v2 3/8] drm/i915: Convert pll macros to _PICK_EVEN_2RANGES
> -Original Message- > From: Intel-gfx On Behalf Of Lucas > De Marchi > Sent: Friday, January 20, 2023 11:35 AM > To: intel-...@lists.freedesktop.org > Cc: De Marchi, Lucas ; dri- > de...@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 3/8] drm/i915: Convert pll macros to > _PICK_EVEN_2RANGES > > Avoid the array lookup, converting the PLL macros after ICL to > _PICK_EVEN_RANGES. This provides the following reduction in code size: > > $ size build64/drivers/gpu/drm/i915/i915.o{.old,.new} > textdata bss dec hex filename > 4027456 1857036984 4220143 4064ef > build64/drivers/gpu/drm/i915/i915.o.old > 4026997 1857036984 4219684 406324 > build64/drivers/gpu/drm/i915/i915.o.new > > At the same time it's safer, avoiding out-of-bounds array access. This > allows to > remove _MMIO_PLL3() that is now unused. > > Signed-off-by: Lucas De Marchi Reviewed-by: Anusha Srivatsa > --- > .../drm/i915/display/intel_display_reg_defs.h | 1 - > drivers/gpu/drm/i915/i915_reg.h | 59 +-- > 2 files changed, 29 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > index 02605418ff08..a4ed1c530799 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > +++ b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > @@ -34,7 +34,6 @@ > #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c)) > #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c)) > #define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c)) > -#define _MMIO_PLL3(pll, ...) _MMIO(_PICK(pll, __VA_ARGS__)) > > /* > * Device info offset array based helpers for groups of registers with > unevenly > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 8da3546d82fb..dd1eb8b10e0e 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7232,13 +7232,15 @@ enum skl_power_gate { > #define PLL_LOCK REG_BIT(30) > #define PLL_POWER_ENABLE REG_BIT(27) > #define PLL_POWER_STATEREG_BIT(26) > -#define ICL_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, > _DPLL1_ENABLE, \ > -_ADLS_DPLL2_ENABLE, > _ADLS_DPLL3_ENABLE) > +#define ICL_DPLL_ENABLE(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 3, > \ > + _DPLL0_ENABLE, > _DPLL1_ENABLE,\ > + > _ADLS_DPLL3_ENABLE, _ADLS_DPLL3_ENABLE)) > > #define _DG2_PLL3_ENABLE 0x4601C > > -#define DG2_PLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, > _DPLL1_ENABLE, \ > -_ADLS_DPLL2_ENABLE, > _DG2_PLL3_ENABLE) > +#define DG2_PLL_ENABLE(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 3, > \ > + _DPLL0_ENABLE, > _DPLL1_ENABLE,\ > + _DG2_PLL3_ENABLE, > _DG2_PLL3_ENABLE)) > > #define TBT_PLL_ENABLE _MMIO(0x46020) > > @@ -7251,8 +7253,9 @@ enum skl_power_gate { > _MG_PLL2_ENABLE) > > /* DG1 PLL */ > -#define DG1_DPLL_ENABLE(pll)_MMIO_PLL3(pll, _DPLL0_ENABLE, > _DPLL1_ENABLE, \ > -_MG_PLL1_ENABLE, > _MG_PLL2_ENABLE) > +#define DG1_DPLL_ENABLE(pll)_MMIO(_PICK_EVEN_2RANGES(pll, 2, > \ > + _DPLL0_ENABLE, > _DPLL1_ENABLE,\ > + _MG_PLL1_ENABLE, > _MG_PLL2_ENABLE)) > > /* ADL-P Type C PLL */ > #define PORTTC1_PLL_ENABLE 0x46038 > @@ -7312,9 +7315,9 @@ enum skl_power_gate { > #define _TGL_DPLL0_CFGCR00x164284 > #define _TGL_DPLL1_CFGCR00x16428C > #define _TGL_TBTPLL_CFGCR0 0x16429C > -#define TGL_DPLL_CFGCR0(pll) _MMIO_PLL3(pll, > _TGL_DPLL0_CFGCR0, \ > - _TGL_DPLL1_CFGCR0, \ > - _TGL_TBTPLL_CFGCR0) > +#define TGL_DPLL_CFGCR0(pll) _MMIO(_PICK_EVEN_2RANGES(pll, 2, > \ > + _TGL_DPLL0_CFGCR0, > _TGL_DPLL1_CFGCR0,\ > + _TGL_TBTPLL_CFGCR0, > _TGL_TBTPLL_CFGCR0)) > #define RKL_DPLL_CFGCR0(pll) _MMIO_PLL(pll, _TGL_DPLL0_CFGCR0, > \ > _TGL_DPLL1_CFGCR0) > > @@ -7327,40 +7330,36 @@ enum skl_power_gate { > #define _TGL_DPLL0_CFGCR10x164288 > #define _TGL_DPLL1_CFGCR10x164290 > #define _TGL_TBTPLL_CFGCR1 0x1642A0 > -#define TGL_DPLL_CFGCR1(pll) _MMIO_PLL3(pll, > _TGL_DPLL0_CFGCR1, \ > -_TGL_DPLL1_CFGCR1, \ > -
RE: [PATCH v2.1] drm/i915: Add _PICK_EVEN_2RANGES()
> -Original Message- > From: De Marchi, Lucas > Sent: Monday, January 23, 2023 9:16 AM > To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: Srivatsa, Anusha ; Jani Nikula > ; De Marchi, Lucas > Subject: [PATCH v2.1] drm/i915: Add _PICK_EVEN_2RANGES() > > It's a constant pattern in the driver to need to use 2 ranges of MMIOs based > on > port, phy, pll, etc. When that happens, instead of using _PICK_EVEN(), _PICK() > needs to be used. Using _PICK() is discouraged due to some reasons like: > > 1) It increases the code size since the array is declared >in each call site > 2) Developers need to be careful not to incur an >out-of-bounds array access > 3) Developers need to be careful that the indexes match the >table. For that it may be that the table needs to contain >holes, making (1) even worse. > > Add a variant of _PICK_EVEN() that works with 2 ranges and selects which one > to use depending on the index value. > > v2: Fix the address expansion in the example (Anusha) > > Signed-off-by: Lucas De Marchi Reviewed-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/i915_reg_defs.h | 28 > 1 file changed, 28 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h > b/drivers/gpu/drm/i915/i915_reg_defs.h > index be43580a6979..bab6a9ec2ddd 100644 > --- a/drivers/gpu/drm/i915/i915_reg_defs.h > +++ b/drivers/gpu/drm/i915/i915_reg_defs.h > @@ -119,6 +119,34 @@ > */ > #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a))) > > +/* > + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets. > + * The first range is used for indexes below @__c_index, and the second > + * range is used for anything above it. Example:: > + * > + * #define _FOO_A0xf000 > + * #define _FOO_B0xf004 > + * #define _FOO_C0xf008 > + * #define _SUPER_FOO_A 0xa000 > + * #define _SUPER_FOO_B 0xa100 > + * #define FOO(x)_MMIO(_PICK_EVEN_RANGES(x, 3, > \ > + * _FOO_A, _FOO_B, > \ > + * _SUPER_FOO_A, _SUPER_FOO_B)) > + * > + * This expands to: > + * 0: 0xf000, > + * 1: 0xf004, > + * 2: 0xf008, > + * 3: 0xa000, > + * 4: 0xa100, > + * 5: 0xa200, > + * ... > + */ > +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) > \ > + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) + > \ > + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : > \ > +_PICK_EVEN((__index) - (__c_index), __c, > __d))) > + > /* > * Given the arbitrary numbers in varargs, pick the 0-based __index'th > number. > * > -- > 2.39.0
RE: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
> -Original Message- > From: Jani Nikula > Sent: Monday, January 23, 2023 3:01 AM > To: De Marchi, Lucas ; Srivatsa, Anusha > > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES() > > On Sat, 21 Jan 2023, Lucas De Marchi wrote: > > On Fri, Jan 20, 2023 at 10:14:19PM -0800, Anusha Srivatsa wrote: > >> > >> > >>> -Original Message- > >>> From: Intel-gfx On Behalf > >>> Of Lucas De Marchi > >>> Sent: Friday, January 20, 2023 11:35 AM > >>> To: intel-...@lists.freedesktop.org > >>> Cc: De Marchi, Lucas ; dri- > >>> de...@lists.freedesktop.org > >>> Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add > >>> _PICK_EVEN_2RANGES() > >>> > >>> It's a constant pattern in the driver to need to use 2 ranges of > >>> MMIOs based on port, phy, pll, etc. When that happens, instead of > >>> using _PICK_EVEN(), _PICK() needs to be used. Using _PICK() is > >>> discouraged > due to some reasons like: > >>> > >>> 1) It increases the code size since the array is declared > >>>in each call site > >>> 2) Developers need to be careful not to incur an > >>>out-of-bounds array access > >>> 3) Developers need to be careful that the indexes match the > >>>table. For that it may be that the table needs to contain > >>>holes, making (1) even worse. > >>> > >>> Add a variant of _PICK_EVEN() that works with 2 ranges and selects > >>> which one to use depending on the index value. > >>> > >>> Signed-off-by: Lucas De Marchi > >>> --- > >>> drivers/gpu/drm/i915/i915_reg_defs.h | 28 > >>> > >>> 1 file changed, 28 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h > >>> b/drivers/gpu/drm/i915/i915_reg_defs.h > >>> index be43580a6979..b7ec87464d69 100644 > >>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h > >>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h > >>> @@ -119,6 +119,34 @@ > >>> */ > >>> #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - > >>> (__a))) > >>> > >>> +/* > >>> + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address > offsets. > >>> + * The first range is used for indexes below @__c_index, and the > >>> +second > >>> + * range is used for anything above it. Example:: > >>> + * > >>> + * #define _FOO_A0xf000 > >>> + * #define _FOO_B0xf004 > >>> + * #define _FOO_C0xf008 > >>> + * #define _SUPER_FOO_A 0xa000 > >>> + * #define _SUPER_FOO_B 0xa100 > >>> + * #define FOO(x)_MMIO(_PICK_EVEN_RANGES(x, 3, > >>> \ > >>> + * _FOO_A, _FOO_B, > >>> \ > >>> + * _SUPER_FOO_A, > >>> _SUPER_FOO_B)) > >>> + * > >>> + * This expands to: > >>> + * 0: 0xf000, > >>> + * 1: 0xf004, > >>> + * 2: 0xf008, > >>> + * 3: 0xa100, > >>You mean 3:0xa000 > > > > doesn't really matter. This is an example of register addresses. They > > don't need to start from 0, it's whatever the hw gives us. > > I think the point is that the example is inconsistent between _SUPER_FOO_A and > "3: 0xa100". Yes. It's the inconsistency with _SUPER_FOO_A that I was pointing out. Anusha > BR, > Jani. > > > > > Lucas De Marchi > > > >> > >>> + * 4: 0xa200, > >>4:0xa100 > >> > >>> + * 5: 0xa300, > >>5:0xa200 > >> > >>Anusha > >>> + * ... > >>> + */ > >>> +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) > >>> \ > >>> + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) + > >>> \ > >>> + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : > >>> \ > >>> +_PICK_EVEN((__index) - (__c_index), __c, > >>> __d))) > >>> + > >>> /* > >>> * Given the arbitrary numbers in varargs, pick the 0-based __index'th > number. > >>> * > >>> -- > >>> 2.39.0 > >> > > -- > Jani Nikula, Intel Open Source Graphics Center
RE: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES()
> -Original Message- > From: Intel-gfx On Behalf Of Lucas > De Marchi > Sent: Friday, January 20, 2023 11:35 AM > To: intel-...@lists.freedesktop.org > Cc: De Marchi, Lucas ; dri- > de...@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 1/8] drm/i915: Add _PICK_EVEN_2RANGES() > > It's a constant pattern in the driver to need to use 2 ranges of MMIOs based > on > port, phy, pll, etc. When that happens, instead of using _PICK_EVEN(), _PICK() > needs to be used. Using _PICK() is discouraged due to some reasons like: > > 1) It increases the code size since the array is declared >in each call site > 2) Developers need to be careful not to incur an >out-of-bounds array access > 3) Developers need to be careful that the indexes match the >table. For that it may be that the table needs to contain >holes, making (1) even worse. > > Add a variant of _PICK_EVEN() that works with 2 ranges and selects which one > to use depending on the index value. > > Signed-off-by: Lucas De Marchi > --- > drivers/gpu/drm/i915/i915_reg_defs.h | 28 > 1 file changed, 28 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h > b/drivers/gpu/drm/i915/i915_reg_defs.h > index be43580a6979..b7ec87464d69 100644 > --- a/drivers/gpu/drm/i915/i915_reg_defs.h > +++ b/drivers/gpu/drm/i915/i915_reg_defs.h > @@ -119,6 +119,34 @@ > */ > #define _PICK_EVEN(__index, __a, __b) ((__a) + (__index) * ((__b) - (__a))) > > +/* > + * Like _PICK_EVEN(), but supports 2 ranges of evenly spaced address offsets. > + * The first range is used for indexes below @__c_index, and the second > + * range is used for anything above it. Example:: > + * > + * #define _FOO_A0xf000 > + * #define _FOO_B0xf004 > + * #define _FOO_C0xf008 > + * #define _SUPER_FOO_A 0xa000 > + * #define _SUPER_FOO_B 0xa100 > + * #define FOO(x)_MMIO(_PICK_EVEN_RANGES(x, 3, > \ > + * _FOO_A, _FOO_B, > \ > + * _SUPER_FOO_A, _SUPER_FOO_B)) > + * > + * This expands to: > + * 0: 0xf000, > + * 1: 0xf004, > + * 2: 0xf008, > + * 3: 0xa100, You mean 3:0xa000 > + * 4: 0xa200, 4:0xa100 > + * 5: 0xa300, 5:0xa200 Anusha > + * ... > + */ > +#define _PICK_EVEN_2RANGES(__index, __c_index, __a, __b, __c, __d) > \ > + (BUILD_BUG_ON_ZERO(!__is_constexpr(__c_index)) + > \ > + ((__index) < (__c_index) ? _PICK_EVEN(__index, __a, __b) : > \ > +_PICK_EVEN((__index) - (__c_index), __c, > __d))) > + > /* > * Given the arbitrary numbers in varargs, pick the 0-based __index'th > number. > * > -- > 2.39.0
RE: [Intel-gfx] [PATCH v2 8/8] drm/i915: Convert PALETTE() to _PICK_EVEN_2RANGES()
> -Original Message- > From: Intel-gfx On Behalf Of Lucas > De Marchi > Sent: Friday, January 20, 2023 11:35 AM > To: intel-...@lists.freedesktop.org > Cc: De Marchi, Lucas ; dri- > de...@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 8/8] drm/i915: Convert PALETTE() to > _PICK_EVEN_2RANGES() > > PALETTE() can use _PICK_EVEN_2RANGES instead of _PICK, which reduces the > size and is safer. > > Signed-off-by: Lucas De Marchi Reviewed-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/i915_reg.h | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 3d6ad4424265..b134a1f357c8 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1734,10 +1734,11 @@ > #define PALETTE_10BIT_BLUE_EXP_MASKREG_GENMASK(7, 6) > #define PALETTE_10BIT_BLUE_MANT_MASK REG_GENMASK(5, 2) > #define PALETTE_10BIT_BLUE_UDW_MASKREG_GENMASK(1, 0) > -#define PALETTE(pipe, i) _MMIO(DISPLAY_MMIO_BASE(dev_priv) + \ > - _PICK((pipe), _PALETTE_A, \ > - _PALETTE_B, _CHV_PALETTE_C) + \ > - (i) * 4) > +#define PALETTE(pipe, i) _MMIO(DISPLAY_MMIO_BASE(dev_priv) + > \ > +_PICK_EVEN_2RANGES(pipe, 2, > \ > + _PALETTE_A, _PALETTE_B, > \ > + _CHV_PALETTE_C, > _CHV_PALETTE_C) + \ > + (i) * 4) > > #define PEG_BAND_GAP_DATA_MMIO(0x14d68) > > -- > 2.39.0
RE: [Intel-gfx] [PATCH v2 7/8] drm/i915: Convert MBUS_ABOX_CTL() to _PICK_EVEN_2RANGES()
> -Original Message- > From: Intel-gfx On Behalf Of Lucas > De Marchi > Sent: Friday, January 20, 2023 11:35 AM > To: intel-...@lists.freedesktop.org > Cc: De Marchi, Lucas ; dri- > de...@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 7/8] drm/i915: Convert MBUS_ABOX_CTL() to > _PICK_EVEN_2RANGES() > > MBUS_ABOX_CTL() can use _PICK_EVEN_2RANGES instead of _PICK, which > reduces the size and is safer. > > Signed-off-by: Lucas De Marchi Looks good! Reviewed-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/i915_reg.h | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index fe6385443c4a..3d6ad4424265 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1040,9 +1040,11 @@ > #define _MBUS_ABOX0_CTL 0x45038 > #define _MBUS_ABOX1_CTL 0x45048 > #define _MBUS_ABOX2_CTL 0x4504C > -#define MBUS_ABOX_CTL(x) _MMIO(_PICK(x, _MBUS_ABOX0_CTL, > \ > - _MBUS_ABOX1_CTL, \ > - _MBUS_ABOX2_CTL)) > +#define MBUS_ABOX_CTL(x) > \ > + _MMIO(_PICK_EVEN_2RANGES(x, 2, > \ > + _MBUS_ABOX0_CTL, _MBUS_ABOX1_CTL, > \ > + _MBUS_ABOX2_CTL, _MBUS_ABOX2_CTL)) > + > #define MBUS_ABOX_BW_CREDIT_MASK (3 << 20) > #define MBUS_ABOX_BW_CREDIT(x) ((x) << 20) > #define MBUS_ABOX_B_CREDIT_MASK (0xF << 16) > -- > 2.39.0
RE: [Intel-gfx] [PATCH v2 6/8] drm/i915: Convert _FIA() to _PICK_EVEN_2RANGES()
> -Original Message- > From: Intel-gfx On Behalf Of Lucas > De Marchi > Sent: Friday, January 20, 2023 11:35 AM > To: intel-...@lists.freedesktop.org > Cc: De Marchi, Lucas ; dri- > de...@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 6/8] drm/i915: Convert _FIA() to > _PICK_EVEN_2RANGES() > > _FIA() can use _PICK_EVEN_2RANGES instead of _PICK, which reduces the size > and is safer. > > Signed-off-by: Lucas De Marchi Reviewed-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/display/intel_mg_phy_regs.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h > b/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h > index 0e8248bce52d..0306ade2bc30 100644 > --- a/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h > +++ b/drivers/gpu/drm/i915/display/intel_mg_phy_regs.h > @@ -142,7 +142,9 @@ > #define FIA1_BASE0x163000 > #define FIA2_BASE0x16E000 > #define FIA3_BASE0x16F000 > -#define _FIA(fia)_PICK((fia), FIA1_BASE, FIA2_BASE, > FIA3_BASE) > +#define _FIA(fia)_PICK_EVEN_2RANGES((fia), 1, > \ > +FIA1_BASE, > FIA1_BASE,\ > +FIA2_BASE, > FIA3_BASE) > #define _MMIO_FIA(fia, off) _MMIO(_FIA(fia) + (off)) > > /* ICL PHY DFLEX registers */ > -- > 2.39.0
RE: [Intel-gfx] [PATCH v2 5/8] drm/i915: Convert PIPE3/PORT3 to _PICK_EVEN_2RANGES()
> -Original Message- > From: Intel-gfx On Behalf Of Lucas > De Marchi > Sent: Friday, January 20, 2023 11:35 AM > To: intel-...@lists.freedesktop.org > Cc: De Marchi, Lucas ; dri- > de...@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 5/8] drm/i915: Convert PIPE3/PORT3 to > _PICK_EVEN_2RANGES() > > Like done for when __var_args__ were used, but size-wise it's also benefitial > to > avoid _PICK() used for 3 ports/pipes: > > $ size build64/drivers/gpu/drm/i915/i915.o{.old,.new} > textdata bss dec hex filename > 4026288 1857036984 4218975 40605f > build64/drivers/gpu/drm/i915/i915.o.old > 4025496 1857036984 4218183 405d47 > build64/drivers/gpu/drm/i915/i915.o.new > > Signed-off-by: Lucas De Marchi Reviewed-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/display/intel_display_reg_defs.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > index f1681e1396b5..755c1ea8225c 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > +++ b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > @@ -13,7 +13,7 @@ > #define VLV_DISPLAY_BASE 0x18 > > /* > - * Named helper wrappers around _PICK_EVEN() and _PICK(). > + * Named helper wrappers around _PICK_EVEN() and _PICK_EVEN_2RANGES(). > */ > #define _PIPE(pipe, a, b)_PICK_EVEN(pipe, a, b) > #define _PLANE(plane, a, b) _PICK_EVEN(plane, a, b) > @@ -29,8 +29,8 @@ > #define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b)) > #define _MMIO_PHY(phy, a, b) _MMIO(_PHY(phy, a, b)) > > -#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c)) > -#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c)) > +#define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK_EVEN_2RANGES(pipe, > 1, a, a, b, c)) > +#define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK_EVEN_2RANGES(pipe, > 1, a, a, b, c)) > > /* > * Device info offset array based helpers for groups of registers with > unevenly > -- > 2.39.0
RE: [Intel-gfx] [PATCH v2 4/8] drm/i915: Replace _MMIO_PHY3() with _PICK_EVEN_2RANGES()
Verified that the new macro evaluates to the right register offsets. Reviewed-by: Anusha Srivatsa > -Original Message- > From: Intel-gfx On Behalf Of Lucas > De Marchi > Sent: Friday, January 20, 2023 11:35 AM > To: intel-...@lists.freedesktop.org > Cc: De Marchi, Lucas ; dri- > de...@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 4/8] drm/i915: Replace _MMIO_PHY3() with > _PICK_EVEN_2RANGES() > > As done previously for pll, also convert users of _PHY3() to > _PICK_EVEN_2RANGES(). Size comparison of i915.o: > > $ size build64/drivers/gpu/drm/i915/i915.o{.old,.new} > textdata bss dec hex filename > 4026997 1857036984 4219684 406324 > build64/drivers/gpu/drm/i915/i915.o.old > 4026288 1857036984 4218975 40605f > build64/drivers/gpu/drm/i915/i915.o.new > > Signed-off-by: Lucas De Marchi > --- > .../drm/i915/display/intel_display_reg_defs.h| 3 --- > drivers/gpu/drm/i915/i915_reg.h | 16 +--- > 2 files changed, 9 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > index a4ed1c530799..f1681e1396b5 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > +++ b/drivers/gpu/drm/i915/display/intel_display_reg_defs.h > @@ -29,11 +29,8 @@ > #define _MMIO_PLL(pll, a, b) _MMIO(_PLL(pll, a, b)) > #define _MMIO_PHY(phy, a, b) _MMIO(_PHY(phy, a, b)) > > -#define _PHY3(phy, ...) _PICK(phy, __VA_ARGS__) > - > #define _MMIO_PIPE3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c)) > #define _MMIO_PORT3(pipe, a, b, c) _MMIO(_PICK(pipe, a, b, c)) > -#define _MMIO_PHY3(phy, a, b, c) _MMIO(_PHY3(phy, a, b, c)) > > /* > * Device info offset array based helpers for groups of registers with > unevenly > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index dd1eb8b10e0e..fe6385443c4a 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -541,9 +541,10 @@ > #define _BXT_PHY0_BASE 0x6C000 > #define _BXT_PHY1_BASE 0x162000 > #define _BXT_PHY2_BASE 0x163000 > -#define BXT_PHY_BASE(phy)_PHY3((phy), _BXT_PHY0_BASE, \ > - _BXT_PHY1_BASE, \ > - _BXT_PHY2_BASE) > +#define BXT_PHY_BASE(phy) > \ > + _PICK_EVEN_2RANGES(phy, 1, > \ > + _BXT_PHY0_BASE, _BXT_PHY0_BASE, > \ > + _BXT_PHY1_BASE, _BXT_PHY2_BASE) > > #define _BXT_PHY(phy, reg) \ > _MMIO(BXT_PHY_BASE(phy) - _BXT_PHY0_BASE + (reg)) @@ -566,13 > +567,14 @@ > #define BXT_PHY_CTL(port)_MMIO_PORT(port, > _BXT_PHY_CTL_DDI_A, \ > > _BXT_PHY_CTL_DDI_B) > > -#define _PHY_CTL_FAMILY_EDP 0x64C80 > #define _PHY_CTL_FAMILY_DDI 0x64C90 > +#define _PHY_CTL_FAMILY_EDP 0x64C80 > #define _PHY_CTL_FAMILY_DDI_C0x64CA0 > #define COMMON_RESET_DIS (1 << 31) > -#define BXT_PHY_CTL_FAMILY(phy) _MMIO_PHY3((phy), > _PHY_CTL_FAMILY_DDI, \ > - > _PHY_CTL_FAMILY_EDP, \ > - > _PHY_CTL_FAMILY_DDI_C) > +#define BXT_PHY_CTL_FAMILY(phy) > \ > + _MMIO(_PICK_EVEN_2RANGES(phy, 1, > \ > + _PHY_CTL_FAMILY_DDI, > _PHY_CTL_FAMILY_DDI, \ > + _PHY_CTL_FAMILY_EDP, > _PHY_CTL_FAMILY_DDI_C)) > > /* BXT PHY PLL registers */ > #define _PORT_PLL_A 0x46074 > -- > 2.39.0
RE: [Intel-gfx] [PATCH v2 2/8] drm/i915: Fix coding style on DPLL*_ENABLE defines
Changes look good. Reviewed-by: Anusha Srivatsa > -Original Message- > From: Intel-gfx On Behalf Of Lucas > De Marchi > Sent: Friday, January 20, 2023 11:35 AM > To: intel-...@lists.freedesktop.org > Cc: De Marchi, Lucas ; dri- > de...@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 2/8] drm/i915: Fix coding style on DPLL*_ENABLE > defines > > Abide by the rules in the top of the header: 2 spaces for bitfield, prefix > offsets > with underscore and prefer the use of REG_BIT(). > > Signed-off-by: Lucas De Marchi > --- > drivers/gpu/drm/i915/i915_reg.h | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 3b2642397b82..8da3546d82fb 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7224,20 +7224,20 @@ enum skl_power_gate { > > ADLS_DPCLKA_DDIK_SEL_MASK) > > /* ICL PLL */ > -#define DPLL0_ENABLE 0x46010 > -#define DPLL1_ENABLE 0x46014 > +#define _DPLL0_ENABLE0x46010 > +#define _DPLL1_ENABLE0x46014 > #define _ADLS_DPLL2_ENABLE 0x46018 > #define _ADLS_DPLL3_ENABLE 0x46030 > -#define PLL_ENABLE (1 << 31) > -#define PLL_LOCK(1 << 30) > -#define PLL_POWER_ENABLE(1 << 27) > -#define PLL_POWER_STATE (1 << 26) > -#define ICL_DPLL_ENABLE(pll) _MMIO_PLL3(pll, DPLL0_ENABLE, > DPLL1_ENABLE, \ > +#define PLL_ENABLE REG_BIT(31) > +#define PLL_LOCK REG_BIT(30) > +#define PLL_POWER_ENABLE REG_BIT(27) > +#define PLL_POWER_STATEREG_BIT(26) > +#define ICL_DPLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, > _DPLL1_ENABLE, \ > _ADLS_DPLL2_ENABLE, > _ADLS_DPLL3_ENABLE) > > #define _DG2_PLL3_ENABLE 0x4601C > > -#define DG2_PLL_ENABLE(pll) _MMIO_PLL3(pll, DPLL0_ENABLE, > DPLL1_ENABLE, \ > +#define DG2_PLL_ENABLE(pll) _MMIO_PLL3(pll, _DPLL0_ENABLE, > +_DPLL1_ENABLE, \ > _ADLS_DPLL2_ENABLE, > _DG2_PLL3_ENABLE) > > #define TBT_PLL_ENABLE _MMIO(0x46020) > @@ -7246,12 +7246,12 @@ enum skl_power_gate { > #define _MG_PLL2_ENABLE 0x46034 > #define _MG_PLL3_ENABLE 0x46038 > #define _MG_PLL4_ENABLE 0x4603C > -/* Bits are the same as DPLL0_ENABLE */ > +/* Bits are the same as _DPLL0_ENABLE */ > #define MG_PLL_ENABLE(tc_port) _MMIO_PORT((tc_port), > _MG_PLL1_ENABLE, \ > _MG_PLL2_ENABLE) > > /* DG1 PLL */ > -#define DG1_DPLL_ENABLE(pll)_MMIO_PLL3(pll, DPLL0_ENABLE, > DPLL1_ENABLE, \ > +#define DG1_DPLL_ENABLE(pll)_MMIO_PLL3(pll, _DPLL0_ENABLE, > _DPLL1_ENABLE, \ > _MG_PLL1_ENABLE, > _MG_PLL2_ENABLE) > > /* ADL-P Type C PLL */ > -- > 2.39.0
RE: [Intel-gfx] [PATCH v2 1/2] drm/i915/dg2: Add preemption changes for Wa_14015141709
Looks good. Reviewed-by: Anusha Srivatsa > -Original Message- > From: Intel-gfx On Behalf Of Matt > Roper > Sent: Friday, March 4, 2022 3:47 PM > To: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 1/2] drm/i915/dg2: Add preemption changes > for Wa_14015141709 > > From: Akeem G Abodunrin > > Starting with DG2, preemption can no longer be controlled using userspace > on a per-context basis. Instead, the hardware only allows us to enable or > disable preemption in a global, system-wide basis. Also, we lose the ability > to specify the preemption granularity (such as batch-level vs command-level > vs object-level). > > v2 (MattR): > - Move debugfs interface to a separate patch. (Jani) > > Cc: Matt Roper > Cc: Prathap Kumar Valsan > Cc: John Harrison > Cc: Joonas Lahtinen > Cc: Jani Nikula > Cc: Tvrtko Ursulin > Signed-off-by: Akeem G Abodunrin > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index beca8735bae5..f695a9c96c8d 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -2310,7 +2310,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, > struct i915_wa_list *wal) >FF_DOP_CLOCK_GATE_DISABLE); > } > > - if (IS_GRAPHICS_VER(i915, 9, 12)) { > + if (HAS_PERCTX_PREEMPT_CTRL(i915)) { > /* > FtrPerCtxtPreemptionGranularityControl:skl,bxt,kbl,cfl,cnl,icl,tgl */ > wa_masked_en(wal, >GEN7_FF_SLICE_CS_CHICKEN1, > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h index 943267393ecb..afefb3bd2714 > 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1396,6 +1396,9 @@ IS_SUBPLATFORM(const struct drm_i915_private > *i915, #define HAS_GUC_DEPRIVILEGE(dev_priv) \ > (INTEL_INFO(dev_priv)->has_guc_deprivilege) > > +#define HAS_PERCTX_PREEMPT_CTRL(i915) \ > + ((GRAPHICS_VER(i915) >= 9) && GRAPHICS_VER_FULL(i915) < > IP_VER(12, > +55)) > + > static inline bool run_as_guest(void) > { > return !hypervisor_is_type(X86_HYPER_NATIVE); > -- > 2.34.1
RE: [v3 1/3] drm/i915/rpl-s: Add PCI IDS for Raptor Lake S
> -Original Message- > From: Hansen, Dave > Sent: Thursday, December 9, 2021 2:27 AM > To: Srivatsa, Anusha ; intel- > g...@lists.freedesktop.org > Cc: x...@kernel.org; dri-devel@lists.freedesktop.org; Ingo Molnar > ; Borislav Petkov ; Dave Hansen > ; Joonas Lahtinen > ; Tvrtko Ursulin > ; Roper, Matthew D > ; Jani Nikula ; > Souza, Jose > Subject: Re: [v3 1/3] drm/i915/rpl-s: Add PCI IDS for Raptor Lake S > > On 12/2/21 10:35 PM, Anusha Srivatsa wrote: > > diff --git a/arch/x86/kernel/early-quirks.c > > b/arch/x86/kernel/early-quirks.c index 391a4e2b8604..fd2d3ab38ebb > > 100644 > > --- a/arch/x86/kernel/early-quirks.c > > +++ b/arch/x86/kernel/early-quirks.c > > @@ -554,6 +554,7 @@ static const struct pci_device_id intel_early_ids[] > __initconst = { > > INTEL_RKL_IDS(_early_ops), > > INTEL_ADLS_IDS(_early_ops), > > INTEL_ADLP_IDS(_early_ops), > > + INTEL_RPLS_IDS(_early_ops), > > }; > > For arch/x86 purposes: > > Acked-by: Dave Hansen Thanks for the ack! Anusha
RE: [v3 0/3] Introduce Raptor Lake S
> -Original Message- > From: Srivatsa, Anusha > Sent: Monday, December 6, 2021 9:59 AM > To: 'Tvrtko Ursulin' ; intel- > g...@lists.freedesktop.org > Cc: x...@kernel.org; dri-devel@lists.freedesktop.org; Ingo Molnar > ; Borislav Petkov ; Dave Hansen > ; Joonas Lahtinen > ; Nikula, Jani > Subject: RE: [v3 0/3] Introduce Raptor Lake S > > > > > -Original Message- > > From: Tvrtko Ursulin > > Sent: Friday, December 3, 2021 2:57 PM > > To: Srivatsa, Anusha ; intel- > > g...@lists.freedesktop.org > > Cc: x...@kernel.org; dri-devel@lists.freedesktop.org; Ingo Molnar > > ; Borislav Petkov ; Dave Hansen > > ; Joonas Lahtinen > > ; Nikula, Jani > > > > Subject: Re: [v3 0/3] Introduce Raptor Lake S > > > > > > On 03/12/2021 06:35, Anusha Srivatsa wrote: > > > Raptor Lake S(RPL-S) is a version 12 Display, Media and Render. For > > > all i915 purposes it is the same as Alder Lake S (ADL-S). > > > > > > The series introduces it as a subplatform of ADL-S. The one > > > difference is the GuC submission which is default on RPL-S but was > > > not the case with ADL-S. > > > > As a side note, not a blocker of any kind, I am slightly disheartened > > but the confusion of ADL_P and ADL_S being separate platforms, but > > then RPL_S is subplatform of ADL_S. Maybe it is just me not being able > > to keep track of things. > > > > > All patches are reviewed. Jani has acked the series. > > > Looking for other acks in order to merge these to respective branches. > > > > Which branches would that be for this series? First two to > > drm-intel-next and last one to drm-intel-gt-next? Is that complication > > needed and/or worth the effort? > > Tvrtko, > All three have to land to drm-intel-next. The last one has dependency on the > first patch and is a trivial change. @Ursulin, Tvrtko @Joonas Lahtinen can I get an ack to merge these into drm-intel-next branch? Anusha > Anusha > > Regards, > > > > Tvrtko > > > > > Cc: x...@kernel.org > > > Cc: dri-devel@lists.freedesktop.org > > > Cc: Ingo Molnar > > > Cc: Borislav Petkov > > > Cc: Dave Hansen > > > Cc: Joonas Lahtinen > > > Cc: Tvrtko Ursulin > > > Acked-by: Jani Nikula > > > > > > Anusha Srivatsa (3): > > >drm/i915/rpl-s: Add PCI IDS for Raptor Lake S > > >drm/i915/rpl-s: Add PCH Support for Raptor Lake S > > >drm/i915/rpl-s: Enable guc submission by default > > > > > > arch/x86/kernel/early-quirks.c | 1 + > > > drivers/gpu/drm/i915/gt/uc/intel_uc.c| 2 +- > > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > > drivers/gpu/drm/i915/i915_pci.c | 1 + > > > drivers/gpu/drm/i915/intel_device_info.c | 7 +++ > > > drivers/gpu/drm/i915/intel_device_info.h | 3 +++ > > > drivers/gpu/drm/i915/intel_pch.c | 1 + > > > drivers/gpu/drm/i915/intel_pch.h | 1 + > > > include/drm/i915_pciids.h| 9 + > > > 9 files changed, 26 insertions(+), 1 deletion(-) > > >
RE: [v3 0/3] Introduce Raptor Lake S
> -Original Message- > From: Tvrtko Ursulin > Sent: Friday, December 3, 2021 2:57 PM > To: Srivatsa, Anusha ; intel- > g...@lists.freedesktop.org > Cc: x...@kernel.org; dri-devel@lists.freedesktop.org; Ingo Molnar > ; Borislav Petkov ; Dave Hansen > ; Joonas Lahtinen > ; Nikula, Jani > Subject: Re: [v3 0/3] Introduce Raptor Lake S > > > On 03/12/2021 06:35, Anusha Srivatsa wrote: > > Raptor Lake S(RPL-S) is a version 12 > > Display, Media and Render. For all i915 purposes it is the same as > > Alder Lake S (ADL-S). > > > > The series introduces it as a subplatform of ADL-S. The one difference > > is the GuC submission which is default on RPL-S but was not the case > > with ADL-S. > > As a side note, not a blocker of any kind, I am slightly disheartened but the > confusion of ADL_P and ADL_S being separate platforms, but then RPL_S is > subplatform of ADL_S. Maybe it is just me not being able to keep track of > things. > > > All patches are reviewed. Jani has acked the series. > > Looking for other acks in order to merge these to respective branches. > > Which branches would that be for this series? First two to drm-intel-next and > last one to drm-intel-gt-next? Is that complication needed and/or worth the > effort? Tvrtko, All three have to land to drm-intel-next. The last one has dependency on the first patch and is a trivial change. Anusha > Regards, > > Tvrtko > > > Cc: x...@kernel.org > > Cc: dri-devel@lists.freedesktop.org > > Cc: Ingo Molnar > > Cc: Borislav Petkov > > Cc: Dave Hansen > > Cc: Joonas Lahtinen > > Cc: Tvrtko Ursulin > > Acked-by: Jani Nikula > > > > Anusha Srivatsa (3): > >drm/i915/rpl-s: Add PCI IDS for Raptor Lake S > >drm/i915/rpl-s: Add PCH Support for Raptor Lake S > >drm/i915/rpl-s: Enable guc submission by default > > > > arch/x86/kernel/early-quirks.c | 1 + > > drivers/gpu/drm/i915/gt/uc/intel_uc.c| 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > drivers/gpu/drm/i915/i915_pci.c | 1 + > > drivers/gpu/drm/i915/intel_device_info.c | 7 +++ > > drivers/gpu/drm/i915/intel_device_info.h | 3 +++ > > drivers/gpu/drm/i915/intel_pch.c | 1 + > > drivers/gpu/drm/i915/intel_pch.h | 1 + > > include/drm/i915_pciids.h| 9 + > > 9 files changed, 26 insertions(+), 1 deletion(-) > >
RE: [PATCH 2/3] drm/i915/dg2: Add initial gt/ctx/engine workarounds
> -Original Message- > From: Roper, Matthew D > Sent: Tuesday, November 2, 2021 3:25 PM > To: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Roper, Matthew D > ; Srivatsa, Anusha > > Subject: [PATCH 2/3] drm/i915/dg2: Add initial gt/ctx/engine workarounds > > Bspec: 54077,68173,54833 > Cc: Anusha Srivatsa > Signed-off-by: Matt Roper Reviewed-by: Anusha Srivatsa > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 278 +++- > drivers/gpu/drm/i915/i915_reg.h | 94 +-- > drivers/gpu/drm/i915/intel_pm.c | 21 +- > 3 files changed, 372 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c > b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 4aaa210fc003..37fd541a9719 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -644,6 +644,42 @@ static void dg1_ctx_workarounds_init(struct > intel_engine_cs *engine, >DG1_HZ_READ_SUPPRESSION_OPTIMIZATION_DISABLE); > } > > +static void dg2_ctx_workarounds_init(struct intel_engine_cs *engine, > + struct i915_wa_list *wal) > +{ > + gen12_ctx_gt_tuning_init(engine, wal); > + > + /* Wa_16011186671:dg2_g11 */ > + if (IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0)) { > + wa_masked_dis(wal, VFLSKPD, > DIS_MULT_MISS_RD_SQUASH); > + wa_masked_en(wal, VFLSKPD, DIS_OVER_FETCH_CACHE); > + } > + > + if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_B0)) { > + /* Wa_14010469329:dg2_g10 */ > + wa_masked_en(wal, GEN11_COMMON_SLICE_CHICKEN3, > + XEHP_DUAL_SIMD8_SEQ_MERGE_DISABLE); > + > + /* > + * Wa_22010465075:dg2_g10 > + * Wa_22010613112:dg2_g10 > + * Wa_14010698770:dg2_g10 > + */ > + wa_masked_en(wal, GEN11_COMMON_SLICE_CHICKEN3, > + GEN12_DISABLE_CPS_AWARE_COLOR_PIPE); > + } > + > + /* Wa_16013271637:dg2 */ > + wa_masked_en(wal, SLICE_COMMON_ECO_CHICKEN1, > + MSC_MSAA_REODER_BUF_BYPASS_DISABLE); > + > + /* Wa_22012532006:dg2 */ > + if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_C0) || > + IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0)) > + wa_masked_en(wal, GEN9_HALF_SLICE_CHICKEN7, > + > DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA); > +} > + > static void fakewa_disable_nestedbb_mode(struct intel_engine_cs *engine, >struct i915_wa_list *wal) > { > @@ -730,7 +766,9 @@ __intel_engine_init_ctx_wa(struct intel_engine_cs > *engine, > if (engine->class != RENDER_CLASS) > goto done; > > - if (IS_XEHPSDV(i915)) > + if (IS_DG2(i915)) > + dg2_ctx_workarounds_init(engine, wal); > + else if (IS_XEHPSDV(i915)) > ; /* noop; none at this time */ > else if (IS_DG1(i915)) > dg1_ctx_workarounds_init(engine, wal); @@ -1343,12 > +1381,117 @@ xehpsdv_gt_workarounds_init(struct intel_gt *gt, struct > i915_wa_list *wal) > GLOBAL_INVALIDATION_MODE); > } > > +static void > +dg2_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal) > +{ > + struct intel_engine_cs *engine; > + int id; > + > + xehp_init_mcr(gt, wal); > + > + /* Wa_14011060649:dg2 */ > + wa_14011060649(gt, wal); > + > + /* > + * Although there are per-engine instances of these registers, > + * they technically exist outside the engine itself and are not > + * impacted by engine resets. Furthermore, they're part of the > + * GuC blacklist so trying to treat them as engine workarounds > + * will result in GuC initialization failure and a wedged GPU. > + */ > + for_each_engine(engine, gt, id) { > + if (engine->class != VIDEO_DECODE_CLASS) > + continue; > + > + /* Wa_16010515920:dg2_g10 */ > + if (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, > STEP_B0)) > + wa_write_or(wal, VDBOX_CGCTL3F18(engine- > >mmio_base), > + ALNUNIT_CLKGATE_DIS); > + } > + > + if (IS_DG2_G10(gt->i915)) { > + /* Wa_22010523718:dg2 */ > + wa_write_or(wal, UNSLICE_UNIT_LEVEL_CLKGATE, > + CG3DDISCFEG_CLKGATE_DIS); > + > + /* Wa_14011006942:dg2 */ > +
RE: [Intel-gfx] [PATCH v10 01/23] drm/dsc: Modify DRM helper to return complete DSC color depth capabilities
>-Original Message- >From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of >Manasi Navare >Sent: Tuesday, November 20, 2018 10:37 AM >To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org >Subject: [Intel-gfx] [PATCH v10 01/23] drm/dsc: Modify DRM helper to return >complete DSC color depth capabilities > >DSC DPCD color depth register advertises its color depth capabilities by >setting >each of the bits that corresponding to a specific color depth. This patch >defines >those specific color depths and adds a helper to return an array of color depth >capabilities. > >v2: >* Simplify the logic (Ville) > >Signed-off-by: Manasi Navare >Cc: Ville Syrjala Implementation looks good. It is used properly in the atomic check patch too... Reviewed-by: Anusha Srivatsa >--- > drivers/gpu/drm/drm_dp_helper.c | 14 -- > include/drm/drm_dp_helper.h | 3 ++- > 2 files changed, 10 insertions(+), 7 deletions(-) > >diff --git a/drivers/gpu/drm/drm_dp_helper.c >b/drivers/gpu/drm/drm_dp_helper.c index 6d483487f2b4..2d6c491a0542 100644 >--- a/drivers/gpu/drm/drm_dp_helper.c >+++ b/drivers/gpu/drm/drm_dp_helper.c >@@ -1428,17 +1428,19 @@ u8 drm_dp_dsc_sink_line_buf_depth(const u8 >dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]) > } > EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth); > >-u8 drm_dp_dsc_sink_max_color_depth(const u8 >dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]) >+int drm_dp_dsc_sink_supported_input_bpcs(const u8 >dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE], >+ u8 dsc_bpc[3]) > { >+ int num_bpc = 0; > u8 color_depth = dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP - >DP_DSC_SUPPORT]; > > if (color_depth & DP_DSC_12_BPC) >- return 12; >+ dsc_bpc[num_bpc++] = 12; > if (color_depth & DP_DSC_10_BPC) >- return 10; >+ dsc_bpc[num_bpc++] = 10; > if (color_depth & DP_DSC_8_BPC) >- return 8; >+ dsc_bpc[num_bpc++] = 8; > >- return 0; >+ return num_bpc; > } >-EXPORT_SYMBOL(drm_dp_dsc_sink_max_color_depth); >+EXPORT_SYMBOL(drm_dp_dsc_sink_supported_input_bpcs); >diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index >3314e91f6eb3..5736c942c85b 100644 >--- a/include/drm/drm_dp_helper.h >+++ b/include/drm/drm_dp_helper.h >@@ -1123,7 +1123,8 @@ drm_dp_is_branch(const u8 >dpcd[DP_RECEIVER_CAP_SIZE]) > u8 drm_dp_dsc_sink_max_slice_count(const u8 >dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE], > bool is_edp); > u8 drm_dp_dsc_sink_line_buf_depth(const u8 >dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]); >-u8 drm_dp_dsc_sink_max_color_depth(const u8 >dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE]); >+int drm_dp_dsc_sink_supported_input_bpcs(const u8 >dsc_dpc[DP_DSC_RECEIVER_CAP_SIZE], >+ u8 dsc_bpc[3]); > > static inline bool > drm_dp_sink_supports_dsc(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]) >-- >2.19.1 > >___ >Intel-gfx mailing list >intel-...@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [v7 1/4] i915/dp/fec: Add fec_enable to the crtc state.
>-Original Message- >From: Navare, Manasi D >Sent: Tuesday, November 6, 2018 6:40 PM >To: Srivatsa, Anusha >Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Ville >Syrjala >; Jani Nikula >Subject: Re: [v7 1/4] i915/dp/fec: Add fec_enable to the crtc state. > >On Tue, Nov 06, 2018 at 04:31:19PM -0800, Anusha Srivatsa wrote: >> For DP 1.4 and above, Display Stream compression can be enabled only >> if Forward Error Correctin can be performed. >> >> Add a crtc state for FEC. Currently, the state is determined by >> platform, DP and DSC being enabled. Moving forward we can use the >> state to have error correction on other scenarios too if needed. >> >> v2: >> - Control compression_enable with the fec_enable parameter in crtc >> state and with intel_dp_supports_fec() >> (Ville) >> >> - intel_dp_can_fec()/intel_dp_supports_fec()(manasi) >> >> v3: Check for FEC support along with setting crtc state. >> >> v4: add checks to intel_dp_source_supports_dsc.(manasi) >> - Move intel_dp_supports_fec() closer to >> intel_dp_supports_dsc() (Anusha) >> >> v5: Move fec check to intel_dp_supports_dsc(Ville) >> >> v6: Remove warning. rebase. >> >> v7: change crtc state to include DP sink and fec capability of >> source.(Manasi) >> >> Suggested-by: Ville Syrjala >> Cc: dri-devel@lists.freedesktop.org >> Cc: Ville Syrjala >> Cc: Jani Nikula >> Cc: Manasi Navare >> Signed-off-by: Anusha Srivatsa >> --- >> drivers/gpu/drm/i915/intel_dp.c | 31 +-- >> drivers/gpu/drm/i915/intel_drv.h | 3 +++ >> 2 files changed, 32 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c >> b/drivers/gpu/drm/i915/intel_dp.c index 73c00c5acf14..f764c45deaab >> 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -545,7 +545,7 @@ intel_dp_mode_valid(struct drm_connector >*connector, >> dsc_slice_count = >> drm_dp_dsc_sink_max_slice_count(intel_dp- >>dsc_dpcd, >> true); >> -} else { >> +} else if (drm_dp_sink_supports_fec(intel_dp->fec_capable)) { >> dsc_max_output_bpp = >> intel_dp_dsc_get_output_bpp(max_link_clock, >> max_lanes, >> @@ -1710,12 +1710,27 @@ struct link_config_limits { >> int min_bpp, max_bpp; >> }; >> >> +static bool intel_dp_source_supports_fec(struct intel_dp *intel_dp, >> + const struct intel_crtc_state >*pipe_config) { >> +struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); >> +struct drm_i915_private *dev_priv = >> +to_i915(dig_port->base.base.dev); >> + >> +return INTEL_GEN(dev_priv) >= 11 && pipe_config->cpu_transcoder != >> +TRANSCODER_A; } >> + >> +static bool intel_dp_supports_fec(struct intel_dp *intel_dp, >> + const struct intel_crtc_state *pipe_config) { >> +return intel_dp_source_supports_fec(intel_dp, pipe_config) && >> +drm_dp_sink_supports_fec(intel_dp->fec_capable); >> +} >> + >> static bool intel_dp_source_supports_dsc(struct intel_dp *intel_dp, >> const struct intel_crtc_state >*pipe_config) { >> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); >> >> -/* FIXME: FEC needed for external DP until then reject DSC on DP */ >> if (!intel_dp_is_edp(intel_dp)) >> return false; > >No point in keeping this !edp condition here, thats supposed to go with FEC >check, move that to where fec_supports is added. > >> >> @@ -1726,6 +1741,9 @@ static bool intel_dp_source_supports_dsc(struct >> intel_dp *intel_dp, static bool intel_dp_supports_dsc(struct intel_dp >> *intel_dp, >>const struct intel_crtc_state *pipe_config) { >> +if (!pipe_config->fec_enable) >> +return false; > >I think its better to use intel_dp_supports_fec(intel_dp, pipe_config) && !edp >instead of pipe_config->fec_enable because pipe_config->fec_enable state will >be set only in dp_compute_config > >> + >> if (!intel_dp_source_supports_dsc(intel_dp, pipe_config) || >> !drm_dp_sink_supports_dsc(intel_dp->dsc
RE: [v6 3/4] i915/dp/fec: Configure the Forward Error Correction bits.
>-Original Message- >From: Navare, Manasi D >Sent: Tuesday, November 6, 2018 2:42 PM >To: Srivatsa, Anusha >Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Singh, >Gaurav K ; Jani Nikula ; >Ville Syrjala >Subject: Re: [v6 3/4] i915/dp/fec: Configure the Forward Error Correction bits. > >On Mon, Nov 05, 2018 at 03:31:49PM -0800, Anusha Srivatsa wrote: >> If FEC is supported, the corresponding DP_TP_CTL register bits have to >> be configured. >> >> The driver has to program the FEC_ENABLE in DP_TP_CTL[30] register and >> wait till FEC_STATUS in DP_TP_CTL[28] is 1. >> Also add the warn message to make sure that the control register is >> already active while enabling FEC. >> >> v2: >> - Change commit message. Configure fec state after >> link training (Manasi, Gaurav) >> - Remove redundent checks (Manasi) >> - Remove the registers that get added automagically (Anusha) >> >> v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state() (Gaurav) >> >> v4: rebased. >> >> v5: >> - Move the code to the proper spot, according to spec.(Ville) >> - Use fec state as a check too. >> >> v6: Pass intel_encoder, instead of intel_dp. (Ville) >> >> Cc: dri-devel@lists.freedesktop.org >> Cc: Gaurav K Singh >> Cc: Jani Nikula >> Cc: Ville Syrjala >> Cc: Manasi Navare >> Signed-off-by: Anusha Srivatsa >> --- >> drivers/gpu/drm/i915/i915_reg.h | 2 ++ >> drivers/gpu/drm/i915/intel_ddi.c | 24 >> 2 files changed, 26 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h index 1a84e8f98e66..209b64d2f27a >> 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -9148,6 +9148,7 @@ enum skl_power_gate { >> #define _DP_TP_CTL_B0x64140 >> #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B) >> #define DP_TP_CTL_ENABLE (1 << 31) >> +#define DP_TP_CTL_FEC_ENABLE (1 << 30) >> #define DP_TP_CTL_MODE_SST (0 << 27) >> #define DP_TP_CTL_MODE_MST (1 << 27) >> #define DP_TP_CTL_FORCE_ACT(1 << 25) >> @@ -9166,6 +9167,7 @@ enum skl_power_gate { >> #define _DP_TP_STATUS_A 0x64044 >> #define _DP_TP_STATUS_B 0x64144 >> #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A, >> _DP_TP_STATUS_B) >> +#define DP_TP_STATUS_FEC_ENABLE_LIVE (1 << 28) >> #define DP_TP_STATUS_IDLE_DONE (1 << 25) >> #define DP_TP_STATUS_ACT_SENT (1 << 24) >> #define DP_TP_STATUS_MODE_STATUS_MST (1 << 23) >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c >> b/drivers/gpu/drm/i915/intel_ddi.c >> index 53a9b31e66a2..fad7385dbd76 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -3065,6 +3065,28 @@ static void intel_dp_sink_set_fec_ready(struct >intel_dp *intel_dp, >> DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n"); } >> >> +static void intel_ddi_enable_fec(struct intel_encoder *encoder, >> + const struct intel_crtc_state *crtc_state) { >> +struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> +enum port port = encoder->port; >> +u32 val; >> + >> +/* FEC support exists for DP 1.4 only */ > >No need for this comment here now since all the platform and sink checks >already done while setting the the fec state. Agreed. > >> +if (!crtc_state->fec_enable) >> +return; >> + >> +val = I915_READ(DP_TP_CTL(port)); >> +val |= DP_TP_CTL_FEC_ENABLE; >> +I915_WRITE(DP_TP_CTL(port), val); > >Do we need a POSTING_READ() here as well? It's used in case of disable fec in >DP_TP_CTL. >May be double check when we need it and when we don't. I don't think we need POSTING_READ, the spec says "wait for FEC_STATUS in DP_TP_CTL[28] is 1. That is why after the i915_WRITE above, I am using intel_Wait_for_register(). Anusha >Manasi > >> + >> +if (intel_wait_for_register(dev_priv, DP_TP_STATUS(port), >> +DP_TP_STATUS_FEC_ENABLE_LIVE, >> +DP_TP_STATUS_FEC_ENABLE_LIVE, >> +1)) >> +
RE: [PATCH v8 07/19] drm/i915/dp: Compute DSC pipe config in atomic check
>-Original Message- >From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] >Sent: Tuesday, November 6, 2018 6:43 AM >To: Navare, Manasi D >Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Jani >Nikula >; Srivatsa, Anusha ; >Singh, Gaurav K >Subject: Re: [PATCH v8 07/19] drm/i915/dp: Compute DSC pipe config in atomic >check > >On Fri, Nov 02, 2018 at 07:09:03PM -0700, Manasi Navare wrote: >> On Fri, Nov 02, 2018 at 02:31:26PM -0700, Manasi Navare wrote: >> > DSC params like the enable, compressed bpp, slice count and >> > dsc_split are added to the intel_crtc_state. These parameters are >> > set based on the requested mode and available link parameters during >> > the pipe configuration in atomic check phase. >> > These values are then later used to populate the remaining DSC and >> > RC parameters before enbaling DSC in atomic commit. >> > >> > v11: >> > * Const crtc_state, reject DSC on DP without FEC (Ville) >> > * Dont set dsc_split to false (Ville) >> > v10: >> > * Add a helper for dp_dsc support (Ville) >> > * Set pipe_config to max bpp, link params for DSC for now (Ville) >> > * Compute bpp - use dp dsc support helper (Ville) >> > v9: >> > * Rebase on top of drm-tip that now uses fast_narrow config for edp >> > (Manasi) >> > v8: >> > * Check for DSC bpc not 0 (manasi) >> > >> > v7: >> > * Fix indentation in compute_m_n (Manasi) >> > >> > v6 (From Gaurav): >> > * Remove function call of intel_dp_compute_dsc_params() and invoke >> > intel_dp_compute_dsc_params() in the patch where it is defined to >> > fix compilation warning (Gaurav) >> > >> > v5: >> > Add drm_dsc_cfg in intel_crtc_state (Manasi) >> > >> > v4: >> > * Rebase on refactoring of intel_dp_compute_config on tip (Manasi) >> > * Add a comment why we need to check PSR while enabling DSC (Gaurav) >> > >> > v3: >> > * Check PPR > max_cdclock to use 2 VDSC instances (Ville) >> > >> > v2: >> > * Add if-else for eDP/DP (Gaurav) >> > >> > Cc: Jani Nikula >> > Cc: Ville Syrjala >> > Cc: Anusha Srivatsa >> > Cc: Gaurav K Singh >> > Signed-off-by: Manasi Navare >> > Reviewed-by: Anusha Srivatsa >> > Acked-by: Jani Nikula >> > --- >> > drivers/gpu/drm/i915/intel_display.c | 20 +++- >> > drivers/gpu/drm/i915/intel_display.h | 3 +- >> > drivers/gpu/drm/i915/intel_dp.c | 167 --- >> > drivers/gpu/drm/i915/intel_dp_mst.c | 2 +- >> > 4 files changed, 166 insertions(+), 26 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_display.c >> > b/drivers/gpu/drm/i915/intel_display.c >> > index b219d5858160..477e53c37353 100644 >> > --- a/drivers/gpu/drm/i915/intel_display.c >> > +++ b/drivers/gpu/drm/i915/intel_display.c >> > @@ -6442,7 +6442,7 @@ static int ironlake_fdi_compute_config(struct >> > intel_crtc *intel_crtc, >> > >> >pipe_config->fdi_lanes = lane; >> > >> > - intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock, >> > + intel_link_compute_m_n(pipe_config->pipe_bpp, 0, lane, >> > +fdi_dotclock, >> > link_bw, _config->fdi_m_n, false); >> > >> >ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, >> > pipe_config); @@ -6679,17 +6679,25 @@ static void >> > compute_m_n(unsigned int m, unsigned int n, } >> > >> > void >> > -intel_link_compute_m_n(int bits_per_pixel, int nlanes, >> > +intel_link_compute_m_n(int bits_per_pixel, uint16_t compressed_bpp, >> > + int nlanes, >> > int pixel_clock, int link_clock, >> > struct intel_link_m_n *m_n, >> > bool constant_n) >> > { >> >m_n->tu = 64; >> > >> > - compute_m_n(bits_per_pixel * pixel_clock, >> > - link_clock * nlanes * 8, >> > - _n->gmch_m, _n->gmch_n, >> > - constant_n); >> > + /* For DSC, Data M/N calculation uses compressed BPP */ >> > + if (compressed_bpp) >> > + compute_m_n(compressed_bpp * pixel_clock, >> > + link_clock * nlanes * 8, >> > + _n->gmch_m, _n->gmch_n, >> > +
RE: [PATCH v8 04/19] drm/dsc: Add helpers for DSC picture parameter set infoframes
>-Original Message- >From: Navare, Manasi D >Sent: Friday, November 2, 2018 2:31 PM >To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org >Cc: Navare, Manasi D ; Jani Nikula >; Ville Syrjala ; >Srivatsa, Anusha ; Harry Wentland > >Subject: [PATCH v8 04/19] drm/dsc: Add helpers for DSC picture parameter set >infoframes > >According to Display Stream compression spec 1.2, the picture parameter set >metadata is sent from source to sink device using the DP Secondary data packet. >An infoframe is formed for the PPS SDP header and PPS SDP payload bytes. >This patch adds helpers to fill the PPS SDP header and PPS SDP payload >according >to the DSC 1.2 specification. > >v7: >* Use BUILD_BUG_ON() to protect changing struct size (Ville) >* Remove typecaseting (Ville) >* Include byteorder.h in drm_dsc.c (Ville) >v6: >* Use proper sequence points for breaking down the assignments (Chris Wilson) >* Use SPDX identifier >v5: >Do not use bitfields for DRM structs (Jani N) >v4: >* Use DSC constants for params that dont change across configurations >v3: >* Add reference to added kernel-docs in >Documentation/gpu/drm-kms-helpers.rst (Daniel Vetter) > >v2: >* Add EXPORT_SYMBOL for the drm functions (Manasi) > >Cc: dri-devel@lists.freedesktop.org >Cc: Jani Nikula >Cc: Ville Syrjala >Cc: Anusha Srivatsa >Cc: Harry Wentland >Signed-off-by: Manasi Navare >Acked-by: Harry Wentland >--- > Documentation/gpu/drm-kms-helpers.rst | 12 ++ > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/drm_dsc.c | 228 ++ > include/drm/drm_dsc.h | 21 +++ > 4 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 >drivers/gpu/drm/drm_dsc.c > >diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm- >kms-helpers.rst >index 4b4dc236ef6f..b422eb8edf16 100644 >--- a/Documentation/gpu/drm-kms-helpers.rst >+++ b/Documentation/gpu/drm-kms-helpers.rst >@@ -232,6 +232,18 @@ MIPI DSI Helper Functions Reference .. kernel-doc:: >drivers/gpu/drm/drm_mipi_dsi.c >:export: > >+Display Stream Compression Helper Functions Reference >+= >+ >+.. kernel-doc:: drivers/gpu/drm/drm_dsc.c >+ :doc: dsc helpers >+ >+.. kernel-doc:: include/drm/drm_dsc.h >+ :internal: >+ >+.. kernel-doc:: drivers/gpu/drm/drm_dsc.c >+ :export: >+ > Output Probing Helper Functions Reference >= > >diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index >576ba985e138..3a3e6fb6d476 100644 >--- a/drivers/gpu/drm/Makefile >+++ b/drivers/gpu/drm/Makefile >@@ -32,7 +32,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o > drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o > drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o > >-drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \ >+drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_dsc.o >+drm_probe_helper.o \ > drm_plane_helper.o drm_dp_mst_topology.o >drm_atomic_helper.o \ > drm_kms_helper_common.o drm_dp_dual_mode_helper.o \ > drm_simple_kms_helper.o drm_modeset_helper.o \ diff --git >a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c new file mode >100644 index ..3a4942c1ae3b >--- /dev/null >+++ b/drivers/gpu/drm/drm_dsc.c >@@ -0,0 +1,228 @@ >+// SPDX-License-Identifier: MIT Nit- In some places the License is within /* */ like comment Is the right way to Add License Identifier? >+/* >+ * Copyright © 2018 Intel Corp >+ * >+ * Author: >+ * Manasi Navare */ >+ >+#include >+#include >+#include >+#include >+#include >+#include >+#include >+ >+/** >+ * DOC: dsc helpers >+ * >+ * These functions contain some common logic and helpers to deal with >+VESA >+ * Display Stream Compression standard required for DSC on Display >+Port/eDP or >+ * MIPI display interfaces. >+ */ >+ >+/** >+ * drm_dsc_dp_pps_header_init() - Initializes the PPS Header >+ * for DisplayPort as per the DP 1.4 spec. >+ * @pps_sdp: Secondary data packet for DSC Picture Parameter Set */ >+void drm_dsc_dp_pps_header_init(struct drm_dsc_pps_infoframe *pps_sdp) >+{ >+ memset(_sdp->pps_header, 0, sizeof(pps_sdp->pps_header)); >+ >+ pps_sdp->pps_header.HB1 = DP_SDP_PPS; >+ pps_sdp->pps_header.HB2 = >DP_SDP_PPS_HEADER_PAYLOAD_BYTES_MINUS_1; >+} >+EXPORT_SYMBOL(drm_dsc_dp_pps_header_init); >+ >+/** >+ * drm_dsc_pps_infoframe_pack() - Populates the DSC PPS infoframe >+ * using the DSC conf
RE: [PATCH v5 28/28] drm/i915/dsc: Force DSC enable if requested by IGT/userspace
From: Navare, Manasi D Sent: Friday, October 05, 2018 4:23 PM To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Navare, Manasi D; Jani Nikula; Ville Syrjala; Srivatsa, Anusha Subject: [PATCH v5 28/28] drm/i915/dsc: Force DSC enable if requested by IGT/userspace Currently the driver will only enable DSC if a certain mode does not fit the available link BW. However IGT/userspace can force DSC enable through dsc support debugfs node to test the DSC functionality if supported by the panel. Cc: Jani Nikula Cc: Ville Syrjala Cc: Anusha Srivatsa Signed-off-by: Manasi Navare Reviewed-by: Anusha Srivatsa --- drivers/gpu/drm/i915/intel_dp.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 52dca5901aa1..987d3b2eb8d3 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2158,12 +2158,13 @@ intel_dp_compute_link_config(struct intel_encoder *encoder, ); /* enable compression if the mode doesn't fit available BW */ - if (!ret) { - DRM_DEBUG_KMS("DP required Link rate %i does not fit available %i\n", - intel_dp_link_required(adjusted_mode->crtc_clock, -pipe_config->pipe_bpp), - intel_dp_max_data_rate(pipe_config->port_clock, -pipe_config->lane_count)); + if (!ret || intel_dp->force_dsc_en) { + if (!ret) + DRM_DEBUG_KMS("DP required Link rate %i does not fit available %i\n", + intel_dp_link_required(adjusted_mode->crtc_clock, + pipe_config->pipe_bpp), + intel_dp_max_data_rate(pipe_config->port_clock, + pipe_config->lane_count)); if (!intel_dp_dsc_compute_config(intel_dp, pipe_config, )) -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Intel-gfx] [PATCH v5 17/28] drm/i915/dsc: Compute Rate Control parameters for DSC
>-Original Message- >From: Navare, Manasi D >Sent: Tuesday, October 23, 2018 11:43 AM >To: Srivatsa, Anusha >Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org >Subject: Re: [Intel-gfx] [PATCH v5 17/28] drm/i915/dsc: Compute Rate Control >parameters for DSC > >On Mon, Oct 22, 2018 at 04:34:59PM -0700, Srivatsa, Anusha wrote: >> >> >> From: Intel-gfx [intel-gfx-boun...@lists.freedesktop.org] on behalf of >> Manasi Navare [manasi.d.nav...@intel.com] >> Sent: Friday, October 05, 2018 4:22 PM >> To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org >> Subject: [Intel-gfx] [PATCH v5 17/28] drm/i915/dsc: Compute Rate >> Control parameters for DSC >> >> From: Gaurav K Singh >> >> This computation of RC params happens in the atomic commit phase >> during compute_config() to validate if display stream compression can >> be enabled for the requested mode. >> >> v5 (From Manasi): >> * Fix dim checkpatch warnings/checks >> v4(From Gaurav): >> * No change.Rebase on drm-tip >> >> v3 (From Gaurav): >> * Rebase on top of Manasi's latest series >> * Return -ve value in case of failure scenarios (Manasi) >> >> Fix review comments from Ville: >> * Remove unnecessary comments >> * Remove unnecessary paranthesis >> * Add comments for few RC params calculations >> >> v2 (From Manasi): >> * Rebase Gaurav's patch from intel-gfx to gfx-internal >> * Use struct drm_dsc_cfg instead of struct intel_dp as a parameter >> >> Cc: Manasi Navare >> Cc: Jani Nikula >> Cc: Ville Syrjala >> Signed-off-by: Gaurav K Singh >> Signed-off-by: Manasi Navare >> --- >> drivers/gpu/drm/i915/intel_vdsc.c | 127 >> ++ >> 1 file changed, 127 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_vdsc.c >> b/drivers/gpu/drm/i915/intel_vdsc.c >> index 351ea7d71c21..594196a9d0f4 100644 >> --- a/drivers/gpu/drm/i915/intel_vdsc.c >> +++ b/drivers/gpu/drm/i915/intel_vdsc.c >> @@ -317,6 +317,130 @@ static int get_column_index_for_rc_params(u8 >bits_per_component) >> } >> } >> >> +static int intel_compute_rc_parameters(struct drm_dsc_config >> +*vdsc_cfg) { >> + unsigned long groups_per_line = 0; >> + unsigned long groups_total = 0; >> + unsigned long num_extra_mux_bits = 0; >> + unsigned long slice_bits = 0; >> + unsigned long hrd_delay = 0; >> + unsigned long final_scale = 0; >> + unsigned long rbs_min = 0; >> + >> + /* Number of groups used to code each line of a slice */ >> + groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width, >> + DSC_RC_PIXELS_PER_GROUP); >> + >> + /* chunksize in Bytes */ >> + vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width * >> + vdsc_cfg->bits_per_pixel, >> + (8 * 16)); >> + >> + if (vdsc_cfg->convert_rgb) >> + num_extra_mux_bits = 3 * (vdsc_cfg->mux_word_size + >> + (4 * vdsc_cfg->bits_per_component >> + 4) >> + - 2); >> + else >> + num_extra_mux_bits = 3 * vdsc_cfg->mux_word_size + >> + (4 * vdsc_cfg->bits_per_component + 4) + >> + 2 * (4 * vdsc_cfg->bits_per_component) - 2; >> + /* Number of bits in one Slice */ >> + slice_bits = 8 * vdsc_cfg->slice_chunk_size * >> + vdsc_cfg->slice_height; >> + >> + while ((num_extra_mux_bits > 0) && >> + ((slice_bits - num_extra_mux_bits) % vdsc_cfg->mux_word_size)) >> + num_extra_mux_bits--; >> + >> + if (groups_per_line < vdsc_cfg->initial_scale_value - 8) >> + vdsc_cfg->initial_scale_value = groups_per_line + 8; >> + >> + /* scale_decrement_interval calculation according to DSC spec 1.11 */ >> + if (vdsc_cfg->initial_scale_value > 8) >> + vdsc_cfg->scale_decrement_interval = groups_per_line / >> + (vdsc_cfg->initial_scale_value - 8); >> + else >> + vdsc_cfg->scale_decrement_interval = >> + DSC_SCALE_DECREMENT_INTERVAL_MAX; >> + >> + vdsc_cfg->final_offs
RE: [PATCH v5 13/28] drm/i915/dp: Compute DSC pipe config in atomic check
From: Navare, Manasi D Sent: Friday, October 05, 2018 4:22 PM To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Navare, Manasi D; Jani Nikula; Ville Syrjala; Srivatsa, Anusha; Singh, Gaurav K Subject: [PATCH v5 13/28] drm/i915/dp: Compute DSC pipe config in atomic check DSC params like the enable, compressed bpp, slice ocunt and s/ocunt/count dsc_split are added to the intel_crtc_state. These parameters are set based on the requested mode and available link parameters during the pipe configuration in atomic check phase. These values are then later used to populate the remaining DSC and RC parameters before enbaling DSC in atomic commit. v9: * Rebase on top of drm-tip that now uses fast_narrow config for edp (Manasi) v8: * Check for DSC bpc not 0 (manasi) v7: * Fix indentation in compute_m_n (Manasi) v6 (From Gaurav): * Remove function call of intel_dp_compute_dsc_params() and invoke intel_dp_compute_dsc_params() in the patch where it is defined to fix compilation warning (Gaurav) v5: Add drm_dsc_cfg in intel_crtc_state (Manasi) v4: * Rebase on refactoring of intel_dp_compute_config on tip (Manasi) * Add a comment why we need to check PSR while enabling DSC (Gaurav) v3: * Check PPR > max_cdclock to use 2 VDSC instances (Ville) v2: * Add if-else for eDP/DP (Gaurav) Cc: Jani Nikula Cc: Ville Syrjala Cc: Anusha Srivatsa Cc: Gaurav K Singh Signed-off-by: Manasi Navare With the typo fixed, Reviewed-by: Anusha Srivatsa --- drivers/gpu/drm/i915/intel_display.c | 20 +++- drivers/gpu/drm/i915/intel_display.h | 3 +- drivers/gpu/drm/i915/intel_dp.c | 170 ++- drivers/gpu/drm/i915/intel_dp_mst.c | 2 +- 4 files changed, 155 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 36434c5359b1..4ebf7c83085c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6532,7 +6532,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, pipe_config->fdi_lanes = lane; - intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock, + intel_link_compute_m_n(pipe_config->pipe_bpp, 0, lane, fdi_dotclock, link_bw, _config->fdi_m_n, false); ret = ironlake_check_fdi_lanes(dev, intel_crtc->pipe, pipe_config); @@ -6767,17 +6767,25 @@ static void compute_m_n(unsigned int m, unsigned int n, } void -intel_link_compute_m_n(int bits_per_pixel, int nlanes, +intel_link_compute_m_n(int bits_per_pixel, uint16_t compressed_bpp, + int nlanes, int pixel_clock, int link_clock, struct intel_link_m_n *m_n, bool constant_n) { m_n->tu = 64; - compute_m_n(bits_per_pixel * pixel_clock, - link_clock * nlanes * 8, - _n->gmch_m, _n->gmch_n, - constant_n); + /* For DSC, Data M/N calculation uses compressed BPP */ + if (compressed_bpp) + compute_m_n(compressed_bpp * pixel_clock, + link_clock * nlanes * 8, + _n->gmch_m, _n->gmch_n, + constant_n); + else + compute_m_n(bits_per_pixel * pixel_clock, + link_clock * nlanes * 8, + _n->gmch_m, _n->gmch_n, + constant_n); compute_m_n(pixel_clock, link_clock, _n->link_m, _n->link_n, diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h index 9fac67e31205..9eaba1bccae8 100644 --- a/drivers/gpu/drm/i915/intel_display.h +++ b/drivers/gpu/drm/i915/intel_display.h @@ -402,7 +402,8 @@ struct intel_link_m_n { (__i)++) \ for_each_if(plane) -void intel_link_compute_m_n(int bpp, int nlanes, +void intel_link_compute_m_n(int bpp, uint16_t compressed_bpp, + int nlanes, int pixel_clock, int link_clock, struct intel_link_m_n *m_n, bool constant_n); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 8e6891356d5b..05300d82b202 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -47,6 +47,8 @@ /* DP DSC small joiner has 2 FIFOs each of 640 x 6 bytes */ #define DP_DSC_MAX_SMALL_JOINER_RAM_BUFFER 61440 +#define DP_DSC_MIN_SUPPORTED_BPC 8 +#define DP_DSC_MAX_SUPPORTED_BPC 10 /* DP DSC throughput values used for slice count calculations KPixels/s */ #define DP_DSC_PEAK_PIXEL_RATE 272 @@ -1894,6 +1896,16 @@ static int in
RE: [Intel-gfx] [PATCH v5 17/28] drm/i915/dsc: Compute Rate Control parameters for DSC
From: Intel-gfx [intel-gfx-boun...@lists.freedesktop.org] on behalf of Manasi Navare [manasi.d.nav...@intel.com] Sent: Friday, October 05, 2018 4:22 PM To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: [Intel-gfx] [PATCH v5 17/28] drm/i915/dsc: Compute Rate Control parameters for DSC From: Gaurav K Singh This computation of RC params happens in the atomic commit phase during compute_config() to validate if display stream compression can be enabled for the requested mode. v5 (From Manasi): * Fix dim checkpatch warnings/checks v4(From Gaurav): * No change.Rebase on drm-tip v3 (From Gaurav): * Rebase on top of Manasi's latest series * Return -ve value in case of failure scenarios (Manasi) Fix review comments from Ville: * Remove unnecessary comments * Remove unnecessary paranthesis * Add comments for few RC params calculations v2 (From Manasi): * Rebase Gaurav's patch from intel-gfx to gfx-internal * Use struct drm_dsc_cfg instead of struct intel_dp as a parameter Cc: Manasi Navare Cc: Jani Nikula Cc: Ville Syrjala Signed-off-by: Gaurav K Singh Signed-off-by: Manasi Navare --- drivers/gpu/drm/i915/intel_vdsc.c | 127 ++ 1 file changed, 127 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c index 351ea7d71c21..594196a9d0f4 100644 --- a/drivers/gpu/drm/i915/intel_vdsc.c +++ b/drivers/gpu/drm/i915/intel_vdsc.c @@ -317,6 +317,130 @@ static int get_column_index_for_rc_params(u8 bits_per_component) } } +static int intel_compute_rc_parameters(struct drm_dsc_config *vdsc_cfg) +{ + unsigned long groups_per_line = 0; + unsigned long groups_total = 0; + unsigned long num_extra_mux_bits = 0; + unsigned long slice_bits = 0; + unsigned long hrd_delay = 0; + unsigned long final_scale = 0; + unsigned long rbs_min = 0; + + /* Number of groups used to code each line of a slice */ + groups_per_line = DIV_ROUND_UP(vdsc_cfg->slice_width, + DSC_RC_PIXELS_PER_GROUP); + + /* chunksize in Bytes */ + vdsc_cfg->slice_chunk_size = DIV_ROUND_UP(vdsc_cfg->slice_width * + vdsc_cfg->bits_per_pixel, + (8 * 16)); + + if (vdsc_cfg->convert_rgb) + num_extra_mux_bits = 3 * (vdsc_cfg->mux_word_size + + (4 * vdsc_cfg->bits_per_component + 4) + - 2); + else + num_extra_mux_bits = 3 * vdsc_cfg->mux_word_size + + (4 * vdsc_cfg->bits_per_component + 4) + + 2 * (4 * vdsc_cfg->bits_per_component) - 2; + /* Number of bits in one Slice */ + slice_bits = 8 * vdsc_cfg->slice_chunk_size * vdsc_cfg->slice_height; + + while ((num_extra_mux_bits > 0) && + ((slice_bits - num_extra_mux_bits) % vdsc_cfg->mux_word_size)) + num_extra_mux_bits--; + + if (groups_per_line < vdsc_cfg->initial_scale_value - 8) + vdsc_cfg->initial_scale_value = groups_per_line + 8; + + /* scale_decrement_interval calculation according to DSC spec 1.11 */ + if (vdsc_cfg->initial_scale_value > 8) + vdsc_cfg->scale_decrement_interval = groups_per_line / + (vdsc_cfg->initial_scale_value - 8); + else + vdsc_cfg->scale_decrement_interval = DSC_SCALE_DECREMENT_INTERVAL_MAX; + + vdsc_cfg->final_offset = vdsc_cfg->rc_model_size - + (vdsc_cfg->initial_xmit_delay * +vdsc_cfg->bits_per_pixel + 8) / 16 + num_extra_mux_bits; + + if (vdsc_cfg->final_offset >= vdsc_cfg->rc_model_size) { + DRM_ERROR("FinalOfs < RcModelSze for this InitialXmitDelay\n"); + return -EINVAL; + } + + final_scale = (vdsc_cfg->rc_model_size * 8) / + (vdsc_cfg->rc_model_size - vdsc_cfg->final_offset); + if (vdsc_cfg->slice_height > 1) + /* +* NflBpgOffset is 16 bit value with 11 fractional bits +* hence we multiply by 2^11 for preserving the +* fractional part +*/ + vdsc_cfg->nfl_bpg_offset = DIV_ROUND_UP((vdsc_cfg->first_line_bpg_offset << 11), + (vdsc_cfg->slice_height - 1)); + else + vdsc_cfg->nfl_bpg_offset = 0; + + /* 2^16 - 1 */ + if (vdsc_cfg->nfl_bpg_offset > 65535) { + DRM_ERROR("NflBpgOffset is too large for this slice height\n"); + return -EINVAL; + } + + /* Number of groups used to code the entire slice */ + groups_total = groups_per_line * vdsc_cfg->slice_height; + + /* slice_bpg_offset is 16 bit value with 11 fractional bits */ +
RE: [PATCH v5 16/28] drm/i915/dsc: Define & Compute VESA DSC params
From: Navare, Manasi D Sent: Friday, October 05, 2018 4:22 PM To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org Cc: Singh, Gaurav K; Jani Nikula; Ville Syrjala; Srivatsa, Anusha; Navare, Manasi D Subject: [PATCH v5 16/28] drm/i915/dsc: Define & Compute VESA DSC params From: Gaurav K Singh This patches does the following: 1. This patch defines all the DSC parameters as per the VESA DSC specification. These are stored in the encoder and used to compute the PPS parameters to be sent to the Sink. 2. Compute all the DSC parameters which are derived from DSC state of intel_crtc_state. 3. Compute all parameters that are VESA DSC specific This computation happens in the atomic check phase during compute_config() to validate if display stream compression can be enabled for the requested mode. v7: (From Manasi) * Dont use signed int for rc_range_params (Manasi) * Mask the range_bpg_offset to use only 6 bits * Add SPDX identifier (Chris Wilson) v6 (From Manasi): * Add a check for line_buf_depth return value (Anusha) * Remove DRM DSC constants to different patch (Manasi) v5 (From Manasi): * Add logic to limit the max line buf depth for DSC 1.1 to 13 as per DSC 1.1 spec * Fix dim checkpatch warnings/checks v4 (From Gaurav): * Rebase on latest drm tip * rename variable name(Manasi) * Populate linebuf_depth variable(Manasi) v3 (From Gaurav): * Rebase my previous patches on top of Manasi's latest patch series * Using >>n rather than /2^n (Manasi) * Change the commit message to explain what the patch is doing(Gaurav) Fixed review comments from Ville: * Don't use macro TWOS_COMPLEMENT * Mention in comment about the source of RC params * Return directly from case statements * Using single asssignment for assigning rc_range_params * Using < Cc: Ville Syrjala Cc: Anusha Srivatsa Cc: Gaurav K Singh Signed-off-by: Gaurav K Singh Signed-off-by: Manasi Navare Co-developed-by: Manasi Navare Configuring the parameters to the right values. Reviewed-by: Anusha Srivatsa --- drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/intel_dp.c | 7 + drivers/gpu/drm/i915/intel_drv.h | 4 + drivers/gpu/drm/i915/intel_vdsc.c | 455 ++ include/drm/drm_dp_helper.h | 3 + 5 files changed, 471 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_vdsc.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index ef1480c14e4e..127efb2948a7 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -154,7 +154,8 @@ i915-y += dvo_ch7017.o \ intel_sdvo.o \ intel_tv.o \ vlv_dsi.o \ - vlv_dsi_pll.o + vlv_dsi_pll.o \ + intel_vdsc.o # Post-mortem debug and GPU hang state capture i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 05300d82b202..89990a96263b 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2073,6 +2073,13 @@ static bool intel_dp_dsc_compute_config(struct intel_dp *intel_dp, return false; } } + if (intel_dp_compute_dsc_params(intel_dp, pipe_config) < 0) { + DRM_ERROR("Cannot compute valid DSC parameters for Input Bpp = %d" + "Compressed BPP = %d\n", + pipe_config->pipe_bpp, + pipe_config->dsc_params.compressed_bpp); + return false; + } pipe_config->dsc_params.compression_enable = true; DRM_DEBUG_KMS("DP DSC computed with Input Bpp = %d " "Compressed Bpp = %d Slice Count = %d\n", diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 443afe97423a..36089c77e493 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1760,6 +1760,10 @@ uint16_t intel_dp_dsc_get_output_bpp(int link_clock, uint8_t lane_count, uint8_t intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp, int mode_clock, int mode_hdisplay); +/* intel_vdsc.c */ +int intel_dp_compute_dsc_params(struct intel_dp *intel_dp, + struct intel_crtc_state *pipe_config); + static inline unsigned int intel_dp_unused_lane_mask(int lane_count) { return ~((1 << lane_count) - 1) & 0xf; diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c new file mode 100644 index ..351ea7d71c21 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_vdsc.c @@ -0,0 +1,455 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2018 Intel Corporation + * + * Author: Gaurav K Singh + * Manasi Navare + */ + +#include +#include +#include "i915_drv.h" +#include &qu
RE: [Intel-gfx] [PATCH v5 15/28] drm/dsc: Define the DSC 1.1 and 1.2 Line Buffer depth constants
From: Intel-gfx [intel-gfx-boun...@lists.freedesktop.org] on behalf of Manasi Navare [manasi.d.nav...@intel.com] Sent: Friday, October 05, 2018 4:22 PM To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: [Intel-gfx] [PATCH v5 15/28] drm/dsc: Define the DSC 1.1 and 1.2 Line Buffer depth constants DSC specification defines linebuf_depth which contains the line buffer bit depth used to generate the bitstream. These values are defined as per Table 4.1 in DSC 1.2 spec v2 (From Manasi): * Rename as MAX_LINEBUF_DEPTH for DSC 1.1 and DSC 1.2 Cc: dri-devel@lists.freedesktop.org Cc: Jani Nikula Cc: Ville Syrjälä Signed-off-by: Gaurav K Singh Signed-off-by: Manasi Navare Reviewed-by: Anusha Srivatsa --- include/drm/drm_dsc.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h index 0e5e3368d645..8562d8ee8161 100644 --- a/include/drm/drm_dsc.h +++ b/include/drm/drm_dsc.h @@ -41,6 +41,9 @@ #define DSC_PPS_RC_RANGE_MINQP_SHIFT 11 #define DSC_PPS_RC_RANGE_MAXQP_SHIFT 6 #define DSC_PPS_NATIVE_420_SHIFT 1 +#define DSC_1_2_MAX_LINEBUF_DEPTH_BITS 16 +#define DSC_1_2_MAX_LINEBUF_DEPTH_VAL 0 +#define DSC_1_1_MAX_LINEBUF_DEPTH_BITS 13 /* Configuration for a single Rate Control model range */ struct dsc_rc_range_parameters { -- 2.18.0 ___ Intel-gfx mailing list intel-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v5 20/28] drm/i915/dp: Configure i915 Picture parameter Set registers during DSC enabling
>-Original Message- >From: Navare, Manasi D >Sent: Tuesday, October 16, 2018 2:03 PM >To: Srivatsa, Anusha >Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Jani >Nikula >; Ville Syrjala >Subject: Re: [PATCH v5 20/28] drm/i915/dp: Configure i915 Picture parameter Set >registers during DSC enabling > >On Tue, Oct 16, 2018 at 12:58:24PM -0700, Srivatsa, Anusha wrote: >> >> >> >-Original Message- >> >From: Navare, Manasi D >> >Sent: Friday, October 5, 2018 4:23 PM >> >To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org >> >Cc: Navare, Manasi D ; Jani Nikula >> >; Ville Syrjala >> >; Srivatsa, Anusha >> > >> >Subject: [PATCH v5 20/28] drm/i915/dp: Configure i915 Picture >> >parameter Set registers during DSC enabling >> > >> >After encoder->pre_enable() hook, after link training sequence is >> >completed, PPS registers for DSC encoder are configured using the DSC >> >state parameters in intel_crtc_state as part of DSC enabling routine >> >in the source. DSC enabling routine is called after >> >encoder->pre_enable() before enbaling the pipe and after >> >compression is enabled on the sink. >> > >> >v3: >> >* Configure Pic_width/2 for each VDSC engine when two VDSC engines >> >per pipe are used (Manasi) >> >* Add DSC slice_row_per_frame in PPS16 (Manasi) >> > >> >v2: >> >* Enable PG2 power well for VDSC on eDP >> > >> >Cc: Jani Nikula >> >Cc: Ville Syrjala >> >Cc: Anusha Srivatsa >> >Signed-off-by: Manasi Navare Some comments inline. >> >> >--- >> > drivers/gpu/drm/i915/i915_drv.h | 2 + >> > drivers/gpu/drm/i915/intel_display.c | 6 + >> > drivers/gpu/drm/i915/intel_vdsc.c| 418 +++ >> > 3 files changed, 426 insertions(+) >> > >> >diff --git a/drivers/gpu/drm/i915/i915_drv.h >> >b/drivers/gpu/drm/i915/i915_drv.h index 93e57b271d3b..b49985f5d08c >> >100644 >> >--- a/drivers/gpu/drm/i915/i915_drv.h >> >+++ b/drivers/gpu/drm/i915/i915_drv.h >> >@@ -3506,6 +3506,8 @@ extern void intel_rps_mark_interactive(struct >> >drm_i915_private *i915, >> > bool interactive); >> > extern bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv, >> > bool enable); >> >+extern void intel_dsc_enable(struct intel_encoder *encoder, >> >+struct intel_crtc_state *crtc_state); >> > >> > int i915_reg_read_ioctl(struct drm_device *dev, void *data, >> >struct drm_file *file); >> >diff --git a/drivers/gpu/drm/i915/intel_display.c >> >b/drivers/gpu/drm/i915/intel_display.c >> >index 4ebf7c83085c..f7cb41775c57 100644 >> >--- a/drivers/gpu/drm/i915/intel_display.c >> >+++ b/drivers/gpu/drm/i915/intel_display.c >> >@@ -5480,6 +5480,12 @@ static void intel_encoders_pre_enable(struct >> >drm_crtc *crtc, >> > >> >if (encoder->pre_enable) >> >encoder->pre_enable(encoder, crtc_state, conn_state); >> >+ >> >+ /* >> >+* Enable and Configure Display Stream Compression in the >> >source >> >+* if enabled in intel_crtc_state. >> >+*/ >> >+ intel_dsc_enable(encoder, crtc_state); >> >} >> > } >> > >> >diff --git a/drivers/gpu/drm/i915/intel_vdsc.c >> >b/drivers/gpu/drm/i915/intel_vdsc.c >> >index 594196a9d0f4..1f2b5dc82f16 100644 >> >--- a/drivers/gpu/drm/i915/intel_vdsc.c >> >+++ b/drivers/gpu/drm/i915/intel_vdsc.c >> >@@ -580,3 +580,421 @@ int intel_dp_compute_dsc_params(struct intel_dp >> >*intel_dp, >> > >> >return 0; >> > } >> >+ >> >+static void intel_configure_pps_for_dsc_encoder(struct intel_encoder >*encoder, >> >+ struct intel_crtc_state >> >*crtc_state) { >> >+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); >> >+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> >+ struct drm_dsc_config *vdsc_cfg = _state->dp_dsc_cfg; >> >+ enum pipe pipe = crtc->pipe; >> >+ u32 pps_val = 0; >> >+ u32 rc_buf_thresh_dword[4]; >> >+ u32 rc_range_params_dword[8]; >> >+ u8
RE: [PATCH v5 20/28] drm/i915/dp: Configure i915 Picture parameter Set registers during DSC enabling
>-Original Message- >From: Navare, Manasi D >Sent: Friday, October 5, 2018 4:23 PM >To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org >Cc: Navare, Manasi D ; Jani Nikula >; Ville Syrjala ; >Srivatsa, Anusha >Subject: [PATCH v5 20/28] drm/i915/dp: Configure i915 Picture parameter Set >registers during DSC enabling > >After encoder->pre_enable() hook, after link training sequence is completed, >PPS >registers for DSC encoder are configured using the DSC state parameters in >intel_crtc_state as part of DSC enabling routine in the source. DSC enabling >routine is called after >encoder->pre_enable() before enbaling the pipe and after >compression is enabled on the sink. > >v3: >* Configure Pic_width/2 for each VDSC engine when two VDSC engines per pipe >are used (Manasi) >* Add DSC slice_row_per_frame in PPS16 (Manasi) > >v2: >* Enable PG2 power well for VDSC on eDP > >Cc: Jani Nikula >Cc: Ville Syrjala >Cc: Anusha Srivatsa >Signed-off-by: Manasi Navare >--- > drivers/gpu/drm/i915/i915_drv.h | 2 + > drivers/gpu/drm/i915/intel_display.c | 6 + > drivers/gpu/drm/i915/intel_vdsc.c| 418 +++ > 3 files changed, 426 insertions(+) > >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >index 93e57b271d3b..b49985f5d08c 100644 >--- a/drivers/gpu/drm/i915/i915_drv.h >+++ b/drivers/gpu/drm/i915/i915_drv.h >@@ -3506,6 +3506,8 @@ extern void intel_rps_mark_interactive(struct >drm_i915_private *i915, > bool interactive); > extern bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv, > bool enable); >+extern void intel_dsc_enable(struct intel_encoder *encoder, >+ struct intel_crtc_state *crtc_state); > > int i915_reg_read_ioctl(struct drm_device *dev, void *data, > struct drm_file *file); >diff --git a/drivers/gpu/drm/i915/intel_display.c >b/drivers/gpu/drm/i915/intel_display.c >index 4ebf7c83085c..f7cb41775c57 100644 >--- a/drivers/gpu/drm/i915/intel_display.c >+++ b/drivers/gpu/drm/i915/intel_display.c >@@ -5480,6 +5480,12 @@ static void intel_encoders_pre_enable(struct >drm_crtc *crtc, > > if (encoder->pre_enable) > encoder->pre_enable(encoder, crtc_state, conn_state); >+ >+ /* >+ * Enable and Configure Display Stream Compression in the >source >+ * if enabled in intel_crtc_state. >+ */ >+ intel_dsc_enable(encoder, crtc_state); > } > } > >diff --git a/drivers/gpu/drm/i915/intel_vdsc.c >b/drivers/gpu/drm/i915/intel_vdsc.c >index 594196a9d0f4..1f2b5dc82f16 100644 >--- a/drivers/gpu/drm/i915/intel_vdsc.c >+++ b/drivers/gpu/drm/i915/intel_vdsc.c >@@ -580,3 +580,421 @@ int intel_dp_compute_dsc_params(struct intel_dp >*intel_dp, > > return 0; > } >+ >+static void intel_configure_pps_for_dsc_encoder(struct intel_encoder *encoder, >+ struct intel_crtc_state >*crtc_state) { >+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); >+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >+ struct drm_dsc_config *vdsc_cfg = _state->dp_dsc_cfg; >+ enum pipe pipe = crtc->pipe; >+ u32 pps_val = 0; >+ u32 rc_buf_thresh_dword[4]; >+ u32 rc_range_params_dword[8]; >+ u8 num_vdsc_instances = (crtc_state->dsc_params.dsc_split) ? 2 : 1; >+ int i = 0; >+ >+ /* Populate PICTURE_PARAMETER_SET_0 registers */ >+ pps_val = DSC_VER_MAJ | vdsc_cfg->dsc_version_minor << >+ DSC_VER_MIN_SHIFT | >+ vdsc_cfg->bits_per_component << DSC_BPC_SHIFT | >+ vdsc_cfg->line_buf_depth << DSC_LINE_BUF_DEPTH_SHIFT; >+ if (vdsc_cfg->block_pred_enable) >+ pps_val |= DSC_BLOCK_PREDICTION; >+ else >+ pps_val &= ~DSC_BLOCK_PREDICTION; >+ if (vdsc_cfg->convert_rgb) >+ pps_val |= DSC_COLOR_SPACE_CONVERSION; >+ else >+ pps_val &= ~DSC_COLOR_SPACE_CONVERSION; >+ if (vdsc_cfg->enable422) >+ pps_val |= DSC_422_ENABLE; >+ else >+ pps_val &= ~DSC_422_ENABLE; >+ if (vdsc_cfg->vbr_enable) >+ pps_val |= DSC_VBR_ENABLE; >+ else >+ pps_val &= ~DSC_VBR_ENABLE; >+ >+ DRM_INFO("PPS0 = 0x%08x\n", pps_val); >+ if (encoder->type == INTEL_OUTPUT_EDP) { >+ I915_WRITE(DSCA_PICTURE_PARAMETER_SET_
RE: [PATCH 14/23] drm/dsc: Define the DSC 1.1 and 1.2 Line Buffer depth constants
>-Original Message- >From: Navare, Manasi D >Sent: Monday, July 30, 2018 7:13 PM >To: intel-...@lists.freedesktop.org >Cc: ville.syrj...@linux.intel.com; jani.nik...@linux.intel.com; Srivatsa, >Anusha >; Singh, Gaurav K ; dri- >de...@lists.freedesktop.org; Navare, Manasi D >Subject: [PATCH 14/23] drm/dsc: Define the DSC 1.1 and 1.2 Line Buffer depth >constants > >From: Gaurav K Singh > >DSC specification defines linebuf_depth which contains the line buffer bit >depth >used to generate the bitstream. >These values are defined as per Table 4.1 in DSC 1.2 spec > >v2 (From Manasi): >* Rename as MAX_LINEBUF_DEPTH for DSC 1.1 and DSC 1.2 > >Cc: dri-devel@lists.freedesktop.org >Cc: Jani Nikula >Cc: Ville Syrjälä >Signed-off-by: Gaurav K Singh >Signed-off-by: Manasi Navare Reviewed-by: Anusha Srivatsa >--- > include/drm/drm_dsc.h | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h index >30adc15..4cfcd03 100644 >--- a/include/drm/drm_dsc.h >+++ b/include/drm/drm_dsc.h >@@ -56,6 +56,9 @@ > #define DSC_PPS_RC_RANGE_MINQP_SHIFT 11 > #define DSC_PPS_RC_RANGE_MAXQP_SHIFT 6 > #define DSC_PPS_NATIVE_420_SHIFT 1 >+#define DSC_1_2_MAX_LINEBUF_DEPTH_BITS16 >+#define DSC_1_2_MAX_LINEBUF_DEPTH_VAL 0 >+#define DSC_1_1_MAX_LINEBUF_DEPTH_BITS13 > > /* Configuration for a single Rate Control model range */ struct >dsc_rc_range_parameters { >-- >2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v2 08/23] drm/dsc: Define VESA Display Stream Compression Capabilities
>-Original Message- >From: Harry Wentland [mailto:harry.wentl...@amd.com] >Sent: Thursday, August 23, 2018 1:01 PM >To: Navare, Manasi D ; intel- >g...@lists.freedesktop.org >Cc: Singh, Gaurav K ; dri- >de...@lists.freedesktop.org; Jani Nikula ; Ville >Syrjala ; Srivatsa, Anusha > >Subject: Re: [PATCH v2 08/23] drm/dsc: Define VESA Display Stream Compression >Capabilities > >On 2018-07-31 05:07 PM, Manasi Navare wrote: >> From: Gaurav K Singh >> >> This defines all the DSC parameters as per the VESA DSC spec that will >> be required for DSC encoder/decoder >> >> v4 (From Manasi) >> * Add the DSC_MUX_WORD_SIZE constants (Manasi) >> >> v3 (From Manasi) >> * Remove the duplicate define (Suggested By:Harry Wentland) >> >> v2: Define this struct in DRM (From Manasi) >> * Changed the data types to u8/u16 instead of unsigned longs (Manasi) >> * Remove driver specific fields (Manasi) >> * Move this struct definition to DRM (Manasi) >> * Define DSC 1.2 parameters (Manasi) >> * Use DSC_NUM_BUF_RANGES (Manasi) >> * Call it drm_dsc_config (Manasi) >> >> Cc: dri-devel@lists.freedesktop.org >> Cc: Jani Nikula >> Cc: Ville Syrjala >> Cc: Anusha Srivatsa >> Cc: Harry Wentland >> Signed-off-by: Manasi Navare >> Signed-off-by: Gaurav K Singh > >Acked-by: Harry Wentland > >Harry Double checked with the Spec, looks good. Reviewed-by: Anusha Srivatsa >> --- >> include/drm/drm_dsc.h | 110 >> ++ >> 1 file changed, 110 insertions(+) >> >> diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h index >> 678e8e6..eda323d 100644 >> --- a/include/drm/drm_dsc.h >> +++ b/include/drm/drm_dsc.h >> @@ -30,6 +30,116 @@ >> >> /* VESA Display Stream Compression DSC 1.2 constants */ >> #define DSC_NUM_BUF_RANGES 15 >> +#define DSC_MUX_WORD_SIZE_8_10_BPC 48 >> +#define DSC_MUX_WORD_SIZE_12_BPC64 >> + >> +/* Configuration for a single Rate Control model range */ struct >> +dsc_rc_range_parameters { >> +/* Min Quantization Parameters allowed for this range */ >> +u8 range_min_qp; >> +/* Max Quantization Parameters allowed for this range */ >> +u8 range_max_qp; >> +/* Bits/group offset to apply to target for this group */ >> +u8 range_bpg_offset; >> +}; >> + >> +struct drm_dsc_config { >> +/* Bits / component for previous reconstructed line buffer */ >> +u8 line_buf_depth; >> +/* Bits per component to code (must be 8, 10, or 12) */ >> +u8 bits_per_component; >> +/* >> + * Flag indicating to do RGB - YCoCg conversion >> + * and back (should be 1 for RGB input) >> + */ >> +bool convert_rgb; >> +u8 slice_count; >> +/* Slice Width */ >> +u16 slice_width; >> +/* Slice Height */ >> +u16 slice_height; >> +/* >> + * 4:2:2 enable mode (from PPS, 4:2:2 conversion happens >> + * outside of DSC encode/decode algorithm) >> + */ >> +bool enable422; >> +/* Picture Width */ >> +u16 pic_width; >> +/* Picture Height */ >> +u16 pic_height; >> +/* Offset to bits/group used by RC to determine QP adjustment */ >> +u8 rc_tgt_offset_high; >> +/* Offset to bits/group used by RC to determine QP adjustment */ >> +u8 rc_tgt_offset_low; >> +/* Bits/pixel target << 4 (ie., 4 fractional bits) */ >> +u16 bits_per_pixel; >> +/* >> + * Factor to determine if an edge is present based >> + * on the bits produced >> + */ >> +u8 rc_edge_factor; >> +/* Slow down incrementing once the range reaches this value */ >> +u8 rc_quant_incr_limit1; >> +/* Slow down incrementing once the range reaches this value */ >> +u8 rc_quant_incr_limit0; >> +/* Number of pixels to delay the initial transmission */ >> +u16 initial_xmit_delay; >> +/* Number of pixels to delay the VLD on the decoder,not including SSM >*/ >> +u16 initial_dec_delay; >> +/* Block prediction enable */ >> +bool block_pred_enable; >> +/* Bits/group offset to use for first line of the slice */ >> +u8 first_line_bpg_offset; >> +/* Value to use for RC model offset at slice start */ >> +u16 initial_offset; >> +/* Thresholds defining each of the buffer ranges */ >> +u16 rc_buf_thresh[DSC_NUM_BUF_RANGES - 1]; >> +/* Parameters for each of the RC rang
RE: [PATCH v2 07/23] drm/dsc: Define Display Stream Compression PPS infoframe
This patch needs to now incorporate the newly added slice_row_per_frame parameter in PPS_16. Anusha >-Original Message- >From: Navare, Manasi D >Sent: Tuesday, July 31, 2018 2:07 PM >To: intel-...@lists.freedesktop.org >Cc: Navare, Manasi D ; Singh, Gaurav K >; dri-devel@lists.freedesktop.org; Jani Nikula >; Ville Syrjala ; >Srivatsa, Anusha ; Harry Wentland > >Subject: [PATCH v2 07/23] drm/dsc: Define Display Stream Compression PPS >infoframe > >This patch defines a new header file for all the DSC 1.2 structures and >creates a >structure for PPS infoframe which will be used to send picture parameter set >secondary data packet for display stream compression. >All the PPS infoframe syntax elements are taken from DSC 1.2 specification from >VESA. > >Cc: Gaurav K Singh >Cc: dri-devel@lists.freedesktop.org >Cc: Jani Nikula >Cc: Ville Syrjala >Cc: Anusha Srivatsa >Cc: Harry Wentland >Signed-off-by: Manasi Navare >--- > include/drm/drm_dsc.h | 365 >++ > 1 file changed, 365 insertions(+) > create mode 100644 include/drm/drm_dsc.h > >diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h new file mode >100644 index 000..678e8e6 >--- /dev/null >+++ b/include/drm/drm_dsc.h >@@ -0,0 +1,365 @@ >+/* >+ * Copyright (C) 2018 Intel Corp. >+ * >+ * Permission is hereby granted, free of charge, to any person >+obtaining a >+ * copy of this software and associated documentation files (the >+"Software"), >+ * to deal in the Software without restriction, including without >+limitation >+ * the rights to use, copy, modify, merge, publish, distribute, >+sublicense, >+ * and/or sell copies of the Software, and to permit persons to whom >+the >+ * Software is furnished to do so, subject to the following conditions: >+ * >+ * The above copyright notice and this permission notice shall be >+included in >+ * all copies or substantial portions of the Software. >+ * >+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >+EXPRESS OR >+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >+MERCHANTABILITY, >+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO >EVENT >+SHALL >+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, >+DAMAGES OR >+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >+OTHERWISE, >+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE >USE >+OR >+ * OTHER DEALINGS IN THE SOFTWARE. >+ * >+ * Authors: >+ * Manasi Navare */ >+ >+#ifndef DRM_DSC_H_ >+#define DRM_DSC_H_ >+ >+#include >+ >+/* VESA Display Stream Compression DSC 1.2 constants */ >+#define DSC_NUM_BUF_RANGES15 >+ >+/** >+ * struct picture_parameter_set - Represents 128 bytes of Picture >+Parameter Set >+ * >+ * The VESA DSC standard defines picture parameter set (PPS) which >+display >+ * stream compression encoders must communicate to decoders. >+ * The PPS is encapsulated in 128 bytes (PPS 0 through PPS 127). The >+fields in >+ * this structure are as per Table 4.1 in Vesa DSC specification v1.1/v1.2. >+ * The PPS fields that span over more than a byte should be stored in >+Big Endian >+ * format. >+ */ >+struct picture_parameter_set { >+ /** >+ * @dsc_version: >+ * PPS0[3:0] - dsc_version_minor: Contains Minor version of DSC >+ * PPS0[7:4] - dsc_version_major: Contains major version of DSC >+ */ >+ u8 dsc_version; >+ /** >+ * @pps_identifier: >+ * PPS1[7:0] - Application specific identifier that can be >+ * used to differentiate between different PPS tables. >+ */ >+ u8 pps_identifier; >+ /** >+ * @pps_reserved: >+ * PPS2[7:0]- RESERVED Byte >+ */ >+ u8 pps_reserved; >+ /** >+ * @pps_3: >+ * PPS3[3:0] - linebuf_depth: Contains linebuffer bit depth used to >+ * generate the bitstream. (0x0 - 16 bits for DSC 1.2, 0x8 - 8 bits, >+ * 0xA - 10 bits, 0xB - 11 bits, 0xC - 12 bits, 0xD - 13 bits, >+ * 0xE - 14 bits for DSC1.2, 0xF - 14 bits for DSC 1.2. >+ * PPS3[7:4] - bits_per_component: Bits per component for the original >+ * pixels of the encoded picture. >+ * 0x0 = 16bpc (allowed only when dsc_version_minor = 0x2) >+ * 0x8 = 8bpc, 0xA = 10bpc, 0xC = 12bpc, 0xE = 14bpc (also >+ * allowed only when dsc_minor_version = 0x2) >+ */ >+ u8 pps_3; >+ /** >+ * @pps_4: >+ * PPS4[1:0] -These are the most significant 2 bits of >+ * compressed BPP bits_per_pixel[9:0] syntax element. >+ * PPS4[2] - vbr_e
RE: [PATCH v2 03/23] drm/dp: DRM DP helper/macros to get DP sink DSC parameters
>-Original Message- >From: Navare, Manasi D >Sent: Tuesday, July 31, 2018 2:07 PM >To: intel-...@lists.freedesktop.org >Cc: Navare, Manasi D ; Singh, Gaurav K >; dri-devel@lists.freedesktop.org; Jani Nikula >; Ville Syrjala ; >Srivatsa, Anusha >Subject: [PATCH v2 03/23] drm/dp: DRM DP helper/macros to get DP sink DSC >parameters > >This patch adds inline functions and helpers for obtaining DP sink's supported >DSC >parameters like DSC sink support, eDP compressed BPP supported, maximum slice >count supported by the sink devices, DSC line buffer bit depth supported on DP >sink, DSC sink maximum color depth by parsing corresponding DPCD registers. > >v4: >* Add helper to give line buf bit depth (Manasi) >v3: >* Use SLICE_CAP_2 for DP (Anusha) >v2: >* Add DSC sink support macro (Jani N) > >Cc: Gaurav K Singh >Cc: dri-devel@lists.freedesktop.org >Cc: Jani Nikula >Cc: Ville Syrjala >Cc: Anusha Srivatsa >Signed-off-by: Manasi Navare Reviewed-by: Anusha Srivatsa >--- > drivers/gpu/drm/drm_dp_helper.c | 89 >+ > include/drm/drm_dp_helper.h | 30 ++ > 2 files changed, 119 insertions(+) > >diff --git a/drivers/gpu/drm/drm_dp_helper.c >b/drivers/gpu/drm/drm_dp_helper.c index 0cccbcb..7dc61d1 100644 >--- a/drivers/gpu/drm/drm_dp_helper.c >+++ b/drivers/gpu/drm/drm_dp_helper.c >@@ -1336,3 +1336,92 @@ int drm_dp_read_desc(struct drm_dp_aux *aux, >struct drm_dp_desc *desc, > return 0; > } > EXPORT_SYMBOL(drm_dp_read_desc); >+ >+/** >+ * DRM DP Helpers for DSC >+ */ >+u8 drm_dp_dsc_sink_max_slice_count(const u8 >dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE], >+ bool is_edp) >+{ >+ u8 slice_cap1 = dsc_dpcd[DP_DSC_SLICE_CAP_1 - DP_DSC_SUPPORT]; >+ >+ if (is_edp) { >+ /* For eDP, register DSC_SLICE_CAPABILITIES_1 NIT : the actual register is DSC_SLICE_CAP_1 gives slice count >*/ >+ if (slice_cap1 & DP_DSC_4_PER_DP_DSC_SINK) >+ return 4; >+ if (slice_cap1 & DP_DSC_2_PER_DP_DSC_SINK) >+ return 2; >+ if (slice_cap1 & DP_DSC_1_PER_DP_DSC_SINK) >+ return 1; >+ } else { >+ /* For DP, use values from DSC_SLICE_CAP_1 and >DSC_SLICE_CAP2 */ ^^^ DSC_SLICE_CAP_2 >+ u8 slice_cap2 = dsc_dpcd[DP_DSC_SLICE_CAP_2 - >DP_DSC_SUPPORT]; >+ >+ if (slice_cap2 & DP_DSC_24_PER_DP_DSC_SINK) >+ return 24; >+ if (slice_cap2 & DP_DSC_20_PER_DP_DSC_SINK) >+ return 20; >+ if (slice_cap2 & DP_DSC_16_PER_DP_DSC_SINK) >+ return 16; >+ if (slice_cap1 & DP_DSC_12_PER_DP_DSC_SINK) >+ return 12; >+ if (slice_cap1 & DP_DSC_10_PER_DP_DSC_SINK) >+ return 10; >+ if (slice_cap1 & DP_DSC_8_PER_DP_DSC_SINK) >+ return 8; >+ if (slice_cap1 & DP_DSC_6_PER_DP_DSC_SINK) >+ return 6; >+ if (slice_cap1 & DP_DSC_4_PER_DP_DSC_SINK) >+ return 4; >+ if (slice_cap1 & DP_DSC_2_PER_DP_DSC_SINK) >+ return 2; >+ if (slice_cap1 & DP_DSC_1_PER_DP_DSC_SINK) >+ return 1; >+ } >+ >+ return 0; >+} >+EXPORT_SYMBOL(drm_dp_dsc_sink_max_slice_count); >+ >+u8 drm_dp_dsc_sink_line_buf_depth(const u8 >+dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]) >+{ >+ u8 line_buf_depth = dsc_dpcd[DP_DSC_LINE_BUF_BIT_DEPTH - >+DP_DSC_SUPPORT]; >+ >+ switch (line_buf_depth & DP_DSC_LINE_BUF_BIT_DEPTH_MASK) { >+ case DP_DSC_LINE_BUF_BIT_DEPTH_9: >+ return 9; >+ case DP_DSC_LINE_BUF_BIT_DEPTH_10: >+ return 10; >+ case DP_DSC_LINE_BUF_BIT_DEPTH_11: >+ return 11; >+ case DP_DSC_LINE_BUF_BIT_DEPTH_12: >+ return 12; >+ case DP_DSC_LINE_BUF_BIT_DEPTH_13: >+ return 13; >+ case DP_DSC_LINE_BUF_BIT_DEPTH_14: >+ return 14; >+ case DP_DSC_LINE_BUF_BIT_DEPTH_15: >+ return 15; >+ case DP_DSC_LINE_BUF_BIT_DEPTH_16: >+ return 16; >+ case DP_DSC_LINE_BUF_BIT_DEPTH_8: >+ return 8; >+ } >+ >+ return 0; >+} >+EXPORT_SYMBOL(drm_dp_dsc_sink_line_buf_depth); >+ >+u8 drm_dp_dsc_sink_max_color_depth(const u8 >+dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]) >+{ >+ switch (dsc_dpcd[DP_DSC_DEC_COLOR_DEPTH_CAP -
RE: [PATCH v2 01/23] drm/dp: Add DP DSC DPCD receiver capability size define and missing SHIFT
>-Original Message- >From: Navare, Manasi D >Sent: Tuesday, July 31, 2018 2:07 PM >To: intel-...@lists.freedesktop.org >Cc: Navare, Manasi D ; dri- >de...@lists.freedesktop.org; Jani Nikula ; Ville >Syrjala ; Srivatsa, Anusha >; Singh, Gaurav K >Subject: [PATCH v2 01/23] drm/dp: Add DP DSC DPCD receiver capability size >define and missing SHIFT > >This patch defines the DP DSC receiver capability size that gives total number >of >DP DSC DPCD registers. >This also adds a missing #defines for DP DSC support missed in the commit id >(ab6a46ea6842ce "Add DPCD definitions for DP 1.4 DSC feature") > >v3: >* MIN_SLICE_WIDTH = 2560 (Anusha) >* Define DP_DSC_SLICE_WIDTH_MULTIPLIER = 320 >v2: >* Add SHIFT define and DECOMPRESSION_EN define misse din prev patch ^^^ "missed in previous" >Cc: dri-devel@lists.freedesktop.org >Cc: Jani Nikula >Cc: Ville Syrjala >Cc: Anusha Srivatsa >Cc: Gaurav K Singh >Signed-off-by: Manasi Navare Other than the typo, the patch looks good. Checked with Spec. Reviewed-by: Anusha Srivatsa >--- > include/drm/drm_dp_helper.h | 6 ++ > 1 file changed, 6 insertions(+) > >diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index >05cc31b..eb0d86c 100644 >--- a/include/drm/drm_dp_helper.h >+++ b/include/drm/drm_dp_helper.h >@@ -230,6 +230,8 @@ > #define DP_DSC_MAX_BITS_PER_PIXEL_LOW 0x067 /* eDP 1.4 */ > > #define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068 /* eDP 1.4 */ >+# define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK (0x3 << 0) # define >+DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8 > > #define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069 > # define DP_DSC_RGB (1 << 0) >@@ -278,6 +280,8 @@ > # define DP_DSC_THROUGHPUT_MODE_1_1000 (14 << 4) > > #define DP_DSC_MAX_SLICE_WIDTH 0x06C >+#define DP_DSC_MIN_SLICE_WIDTH_VALUE2560 >+#define DP_DSC_SLICE_WIDTH_MULTIPLIER 320 > > #define DP_DSC_SLICE_CAP_2 0x06D > # define DP_DSC_16_PER_DP_DSC_SINK (1 << 0) >@@ -476,6 +480,7 @@ > # define DP_AUX_FRAME_SYNC_VALID (1 << 0) > > #define DP_DSC_ENABLE 0x160 /* DP 1.4 */ >+# define DP_DECOMPRESSION_EN(1 << 0) > > #define DP_PSR_EN_CFG 0x170 /* XXX 1.2? */ > # define DP_PSR_ENABLE(1 << 0) >@@ -962,6 +967,7 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 >link_status[DP_LINK_STATUS_SI > > #define DP_BRANCH_OUI_HEADER_SIZE 0xc > #define DP_RECEIVER_CAP_SIZE 0xf >+#define DP_DSC_RECEIVER_CAP_SIZE0xf > #define EDP_PSR_RECEIVER_CAP_SIZE 2 > #define EDP_DISPLAY_CTL_CAP_SIZE 3 > >-- >2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 01/23] drm/dp: Add DP DSC DPCD receiver capability size define and missing SHIFT
>-Original Message- >From: Navare, Manasi D >Sent: Monday, July 30, 2018 7:13 PM >To: intel-...@lists.freedesktop.org >Cc: ville.syrj...@linux.intel.com; jani.nik...@linux.intel.com; Srivatsa, >Anusha >; Singh, Gaurav K ; >Navare, Manasi D ; dri- >de...@lists.freedesktop.org >Subject: [PATCH 01/23] drm/dp: Add DP DSC DPCD receiver capability size define >and missing SHIFT > >This patch defines the DP DSC receiver capability size that gives total number >of >DP DSC DPCD registers. >This also adds a missing #defines for DP DSC support missed in the commit id >(ab6a46ea6842ce "Add DPCD definitions for DP 1.4 DSC feature") > >v3: >* MIN_SLICE_WIDTH = 2560 (Anusha) >* Define DP_DSC_SLICE_WIDTH_MULTIPLIER = 320 >v2: >* Add SHIFT define and DECOMPRESSION_EN define misse din prev patch missed in previous >Cc: dri-devel@lists.freedesktop.org >Cc: Jani Nikula >Cc: Ville Syrjala >Cc: Anusha Srivatsa >Cc: Gaurav K Singh >Signed-off-by: Manasi Navare But that apart, checked with spec. Changes look good. Reviewed-by: Anusha Srivatsa >--- > include/drm/drm_dp_helper.h | 6 ++ > 1 file changed, 6 insertions(+) > >diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index >05cc31b..eb0d86c 100644 >--- a/include/drm/drm_dp_helper.h >+++ b/include/drm/drm_dp_helper.h >@@ -230,6 +230,8 @@ > #define DP_DSC_MAX_BITS_PER_PIXEL_LOW 0x067 /* eDP 1.4 */ > > #define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068 /* eDP 1.4 */ >+# define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK (0x3 << 0) # define >+DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8 > > #define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069 > # define DP_DSC_RGB (1 << 0) >@@ -278,6 +280,8 @@ > # define DP_DSC_THROUGHPUT_MODE_1_1000 (14 << 4) > > #define DP_DSC_MAX_SLICE_WIDTH 0x06C >+#define DP_DSC_MIN_SLICE_WIDTH_VALUE2560 >+#define DP_DSC_SLICE_WIDTH_MULTIPLIER 320 > > #define DP_DSC_SLICE_CAP_2 0x06D > # define DP_DSC_16_PER_DP_DSC_SINK (1 << 0) >@@ -476,6 +480,7 @@ > # define DP_AUX_FRAME_SYNC_VALID (1 << 0) > > #define DP_DSC_ENABLE 0x160 /* DP 1.4 */ >+# define DP_DECOMPRESSION_EN(1 << 0) > > #define DP_PSR_EN_CFG 0x170 /* XXX 1.2? */ > # define DP_PSR_ENABLE(1 << 0) >@@ -962,6 +967,7 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 >link_status[DP_LINK_STATUS_SI > > #define DP_BRANCH_OUI_HEADER_SIZE 0xc > #define DP_RECEIVER_CAP_SIZE 0xf >+#define DP_DSC_RECEIVER_CAP_SIZE0xf > #define EDP_PSR_RECEIVER_CAP_SIZE 2 > #define EDP_DISPLAY_CTL_CAP_SIZE 3 > >-- >2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for Geminilake
>-Original Message- >From: Vivi, Rodrigo >Sent: Friday, April 20, 2018 11:04 AM >To: Jani Nikula <jani.nik...@linux.intel.com> >Cc: Srivatsa, Anusha <anusha.sriva...@intel.com>; Ian W MORRISON ><ianwmorri...@gmail.com>; airl...@linux.ie; Greg KH ><gre...@linuxfoundation.org>; intel-...@lists.freedesktop.org; linux- >ker...@vger.kernel.org; sta...@vger.kernel.org; dri- >de...@lists.freedesktop.org; Wajdeczko, Michal <michal.wajdec...@intel.com> >Subject: Re: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for >Geminilake > >On Tue, Apr 17, 2018 at 12:02:52PM +0300, Jani Nikula wrote: >> On Mon, 16 Apr 2018, "Srivatsa, Anusha" <anusha.sriva...@intel.com> wrote: >> >>-Original Message- >> >>From: Jani Nikula [mailto:jani.nik...@linux.intel.com] >> >>Sent: Wednesday, April 11, 2018 5:27 AM >> >>To: Ian W MORRISON <ianwmorri...@gmail.com> >> >>Cc: Vivi, Rodrigo <rodrigo.v...@intel.com>; Srivatsa, Anusha >> >><anusha.sriva...@intel.com>; Wajdeczko, Michal >> >><michal.wajdec...@intel.com>; Greg KH <gre...@linuxfoundation.org>; >> >>airl...@linux.ie; joonas.lahti...@linux.intel.com; >> >>linux-ker...@vger.kernel.org; sta...@vger.kernel.org; >> >>intel-...@lists.freedesktop.org; dri- de...@lists.freedesktop.org >> >>Subject: Re: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE >> >>for Geminilake >> >> >> >>On Wed, 11 Apr 2018, Ian W MORRISON <ianwmorri...@gmail.com> wrote: >> >>> >> >>> >> >>>> >> >>>> NAK on indiscriminate Cc: stable. There are zero guarantees that >> >>>> older kernels will work with whatever firmware you throw at them. >> >>>> >> >>> >> >>> I included 'Cc: stable' so the patch would get added to the v4.16 >> >>> and >> >>> v4.15 kernels which I have tested with the patch. I found that >> >>> earlier kernels didn't support the 'linux-firmware' package >> >>> required to get wifi working on Intel's new Gemini Lake NUC. >> >> >> >>You realize that this patch should have nothing to do with wifi? >> >> >> >>Rodrigo, Anusha, if you think Cc: stable is appropriate, please >> >>indicate the specific versions of stable it is appropriate for. >> > >> > Hi Jani, >> > >> > The stable kernel version is 4.12 and beyond. >> > It is appropriate to add the CC: stable in my opinion >> >> Who tested the firmware with v4.12 and later? We only have the CI >> results against *current* drm-tip. We don't even know about v4.16. >> > >I understand your concerns, but the problem was that our old process was a bit >(lot?) messed and there was the unreliable time until the firmware really >lands on >linux-firmware.git. So MODULE_FIRMWARE call was only added after firmware >was really there on firmware repository but it wasn't about the testing. > >In other words, the bump version patch was merged after tested, but >MODULE_FIRMWARE was left behind because firmware blob took a while to get >pulled into linux-firmware.git and we end up forgetting to add it there. > >In my opinion it should be safe to add the MODULE_FIRMWARE there based on >the tests from when the version was bumped. Luis, Elio, can you guys confirm that this firmware is tested and healthy? And also, give a tested-by to this patch please? Thanks, Anusha >> I'm not going to ack and take responsibility for the stable backports >> unless someone actually comes forward with credible Tested-bys. >> >> BR, >> Jani. >> >> >> > >> > Anusha >> >>BR, >> >>Jani. >> >> >> >>> >> >>>> >> >>>> PS. How is this a "RESEND"? I haven't seen this before. >> >>>> >> >>> >> >>> It is a 'RESEND' for that very reason. I initially sent the patch >> >>> to the same people as a similar patch >> >>> (https://patchwork.kernel.org/patch/10143637/) however after >> >>> realising this omitted required addresses I added them and resent the >patch. >> >>> >> >>> Best regards, >> >>> Ian >> >> >> >>-- >> >>Jani Nikula, Intel Open Source Technology Center >> >> -- >> Jani Nikula, Intel Open Source Technology Center >> ___ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for Geminilake
>-Original Message- >From: Jani Nikula [mailto:jani.nik...@linux.intel.com] >Sent: Wednesday, April 11, 2018 5:27 AM >To: Ian W MORRISON <ianwmorri...@gmail.com> >Cc: Vivi, Rodrigo <rodrigo.v...@intel.com>; Srivatsa, Anusha ><anusha.sriva...@intel.com>; Wajdeczko, Michal ><michal.wajdec...@intel.com>; Greg KH <gre...@linuxfoundation.org>; >airl...@linux.ie; joonas.lahti...@linux.intel.com; >linux-ker...@vger.kernel.org; >sta...@vger.kernel.org; intel-...@lists.freedesktop.org; dri- >de...@lists.freedesktop.org >Subject: Re: [RESEND PATCH 1/1] drm/i915/glk: Add MODULE_FIRMWARE for >Geminilake > >On Wed, 11 Apr 2018, Ian W MORRISON <ianwmorri...@gmail.com> wrote: >> >> >>> >>> NAK on indiscriminate Cc: stable. There are zero guarantees that >>> older kernels will work with whatever firmware you throw at them. >>> >> >> I included 'Cc: stable' so the patch would get added to the v4.16 and >> v4.15 kernels which I have tested with the patch. I found that earlier >> kernels didn't support the 'linux-firmware' package required to get >> wifi working on Intel's new Gemini Lake NUC. > >You realize that this patch should have nothing to do with wifi? > >Rodrigo, Anusha, if you think Cc: stable is appropriate, please indicate the >specific >versions of stable it is appropriate for. Hi Jani, The stable kernel version is 4.12 and beyond. It is appropriate to add the CC: stable in my opinion Anusha >BR, >Jani. > >> >>> >>> PS. How is this a "RESEND"? I haven't seen this before. >>> >> >> It is a 'RESEND' for that very reason. I initially sent the patch to >> the same people as a similar patch >> (https://patchwork.kernel.org/patch/10143637/) however after realising >> this omitted required addresses I added them and resent the patch. >> >> Best regards, >> Ian > >-- >Jani Nikula, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH v2 4/5] drm/dp: Macro for DSC eDP Output BPP
Looks good to me. >-Original Message- >From: Navare, Manasi D >Sent: Thursday, January 4, 2018 12:23 AM >To: gfx-internal-de...@eclists.intel.com >Cc: Navare, Manasi D <manasi.d.nav...@intel.com>; Jani Nikula ><jani.nik...@linux.intel.com>; Ville Syrjala <ville.syrj...@linux.intel.com>; >Srivatsa, Anusha <anusha.sriva...@intel.com>; dri-devel@lists.freedesktop.org >Subject: [PATCH v2 4/5] drm/dp: Macro for DSC eDP Output BPP > >This returns the maximum output BPP supported by the eDP panel by reading the >MAX_SUPPORTED_BPP DSC DPCD register. > >Cc: Jani Nikula <jani.nik...@linux.intel.com> >Cc: Ville Syrjala <ville.syrj...@linux.intel.com> >Cc: Anusha Srivatsa <anusha.sriva...@intel.com> >Cc: dri-devel@lists.freedesktop.org >Signed-off-by: Manasi Navare <manasi.d.nav...@intel.com> Reviewed-by: Anusha Srivatsa<anusha.sriva...@intel.com> >--- > include/drm/drm_dp_helper.h | 10 ++ > 1 file changed, 10 insertions(+) > >diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index >06e41b2..4e8f357 100644 >--- a/include/drm/drm_dp_helper.h >+++ b/include/drm/drm_dp_helper.h >@@ -980,6 +980,16 @@ drm_dp_is_branch(const u8 >dpcd[DP_RECEIVER_CAP_SIZE]) > return dpcd[DP_DOWNSTREAMPORT_PRESENT] & >DP_DWN_STRM_PORT_PRESENT; } > >+/* DP/eDP DSC support */ >+static inline u16 >+drm_edp_dsc_get_output_bpp(const u8 >dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]) >+{ >+ return dsc_dpcd[DP_DSC_MAX_BITS_PER_PIXEL_LOW - >DP_DSC_SUPPORT] | >+ (dsc_dpcd[DP_DSC_MAX_BITS_PER_PIXEL_HI - >DP_DSC_SUPPORT] & >+ DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK << >+ DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT); >+} >+ > /* > * DisplayPort AUX channel > */ >-- >2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm: Add DPCD definitions for DP 1.4 FEC feature
>-Original Message- >From: Navare, Manasi D >Sent: Thursday, December 21, 2017 12:36 PM >To: Srivatsa, Anusha <anusha.sriva...@intel.com> >Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Ville >Syrjala ><ville.syrj...@linux.intel.com>; Jani Nikula <jani.nik...@linux.intel.com> >Subject: Re: [PATCH] drm: Add DPCD definitions for DP 1.4 FEC feature > >On Mon, Nov 27, 2017 at 04:55:44PM -0800, Anusha Srivatsa wrote: >> Forward Error Correction is supported on DP 1.4. >> This patch adds corresponding DPCD register definitions. >> >> v2: Add dri-devel to the CC list >> >> Cc: dri-devel@lists.freedesktop.org >> Cc: Ville Syrjala <ville.syrj...@linux.intel.com> >> Cc: Jani Nikula <jani.nik...@linux.intel.com> >> Cc: Manasi Navare <manasi.d.nav...@intel.com> >> Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com> >> --- >> include/drm/drm_dp_helper.h | 29 + >> 1 file changed, 29 insertions(+) >> >> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h >> index da58a42..bc816ea 100644 >> --- a/include/drm/drm_dp_helper.h >> +++ b/include/drm/drm_dp_helper.h >> @@ -284,6 +284,35 @@ >> # define DP_DSC_BITS_PER_PIXEL_1_2 0x3 >> # define DP_DSC_BITS_PER_PIXEL_10x4 >> >> +/* DP Forward error Correction Registers */ >> +#define DP_FEC_CAPABILITY 0x090 >> +# define DP_FEC_CAPABLE (1 << 0) >> +# define DP_FEC_UNCORR_BLK_ERROR_COUNT_CAP (1 << 1) >> +# define DP_FEC_CORR_BLK_ERROR_COUNT_CAP(1 << 2) >> +# define DP_FEC_BIT_ERROR_COUNT_CAP (1 << 3) >> + >> +#define DP_FEC_CONFIGURATION0x120 >> +# define DP_FEC_READY (1 << 0) >> +# define DP_FEC_ERR_COUNT_DIS (0 << 1) >> +# define DP_FEC_UNCORR_BLK_ERROR_COUNT (1 << 1) >> +# define DP_FEC_CORR_BLK_ERROR_COUNT(2 << 1) >> +# define DP_FEC_BIT_ERROR_COUNT (3 << 1) > >These above values indicate the value of FEC_ERROR_COUNT_SEL. >I think we would need a mask for FEC_ERROR_COUNT_SEL field so that we can >read this field as drm_dpcd_read(DP_FEC_CONFIGURATION) & >FEC_ERROR_COUNT_SEL_MASK and then compare this to each of the values for >that field. >So we would need an extra #define for the MASK Sounds good >> +# define DP_FEC_LANE_0_SELECT (0 << 4) >> +# define DP_FEC_LANE_1_SELECT (1 << 4) >> +# define DP_FEC_LANE_2_SELECT (2 << 4) >> +# define DP_FEC_LANE_3_SELECT (3 << 4) >> + >> +#define DP_FEC_STATUS 0x280 >> +# define DP_FEC_EN_DETECTED (1 << 0) > >I think better name would be DP_FEC_DECODE_EN_DETECTED since this refers to >FEC_DECODE_EN link symbol sequence Now that you mentioned, I noticed that's the name used in spec too. I wonder why I went ahead with the above name. Shall change it. Thanks. >> +# define DP_FEC_DEC_DETECTED(1 << 1) > >And this should be DP_FEC_DECODE_DIS_DETECTED since this refers to >FEC_DECODE_DIS link symbol sequence > >> + >> +#define DP_FEC_ERROR_COUNT_10x0281 >> +# define DP_FEC_ERR_COUNT_7_0(err_count)(err_count << 0) > >So this is a RO register and so we wont be writing the err_count by passing it >as >an argument as above. >And this is the entire reister that indicates the LSB of ERR_COUNT you can just >rename the register as DP_FEC_ERR_COUNT_LSB So while reading you just pass >this register address and get LSB into a variable. Yes, good point. > >> + >> +#define DP_FEC_ERROR_COUNT_20x0282 >> +# define DP_FEC_ERR_COUNT_14_8(err_count) (err_count << 0) > >This could be DP_FEC_ERR_COUNT_MSB_MASK and SHIFT should be 8 since you >want to put this value at the 8th bit of a 16 bit value. This helps thanks! >> +# define DP_FEC_ERR_COUNT_VALID (1 << 7) > >Everything else looks good. Thanks Manasi. I will incorporate these changes in the next revision. Anusha >Manasi > >> + >> #define DP_PSR_SUPPORT 0x070 /* XXX 1.2? */ >> # define DP_PSR_IS_SUPPORTED1 >> # define DP_PSR2_IS_SUPPORTED 2 /* eDP 1.4 */ >> -- >> 2.7.4 >> ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel