[Intel-gfx] [PATCH 07/10] drm/i915/dmc: add support to load dmc_header version 3
Lucas De Marchi
lucas.demarchi at intel.com
Thu May 23 19:25:34 UTC 2019
On Thu, May 23, 2019 at 11:57:19AM -0700, Rodrigo Vivi wrote:
>On Thu, May 23, 2019 at 01:24:17AM -0700, Lucas De Marchi wrote:
>> Main difference is that now there are up to 20 MMIOs that can be set and
>> a lot of noise due to the struct changing the fields in the middle.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 4 +-
>> drivers/gpu/drm/i915/intel_csr.c | 107 +++++++++++++++++++++++--------
>> 2 files changed, 83 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 1ad3818d2676..04a6b59256fd 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -354,8 +354,8 @@ struct intel_csr {
>> u32 dmc_fw_size; /* dwords */
>> u32 version;
>> u32 mmio_count;
>> - i915_reg_t mmioaddr[8];
>> - u32 mmiodata[8];
>> + i915_reg_t mmioaddr[20];
>> + u32 mmiodata[20];
>> u32 dc_state;
>> u32 allowed_dc_mask;
>> intel_wakeref_t wakeref;
>> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
>> index 20dd4bd5feae..ad4ee55a8c5e 100644
>> --- a/drivers/gpu/drm/i915/intel_csr.c
>> +++ b/drivers/gpu/drm/i915/intel_csr.c
>> @@ -72,6 +72,8 @@ MODULE_FIRMWARE(BXT_CSR_PATH);
>> #define CSR_DEFAULT_FW_OFFSET 0xFFFFFFFF
>> #define PACKAGE_MAX_FW_INFO_ENTRIES 20
>> #define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>> +#define DMC_V1_MAX_MMIO_COUNT 8
>> +#define DMC_V3_MAX_MMIO_COUNT 20
>>
>> struct intel_css_header {
>> /* 0x09 for DMC */
>> @@ -143,7 +145,7 @@ struct intel_package_header {
>> u32 num_entries;
>> } __packed;
>>
>> -struct intel_dmc_header {
>> +struct intel_dmc_header_base {
>> /* always value would be 0x40403E3E */
>> u32 signature;
>>
>> @@ -164,22 +166,47 @@ struct intel_dmc_header {
>>
>> /* Major Minor version */
>> u32 fw_version;
>> +} __packed;
>> +
>> +struct intel_dmc_header_v1 {
>> + struct intel_dmc_header_base base;
>>
>> /* Number of valid MMIO cycles present. */
>> u32 mmio_count;
>>
>> /* MMIO address */
>> - u32 mmioaddr[8];
>> + u32 mmioaddr[DMC_V1_MAX_MMIO_COUNT];
>>
>> /* MMIO data */
>> - u32 mmiodata[8];
>> + u32 mmiodata[DMC_V1_MAX_MMIO_COUNT];
>>
>> /* FW filename */
>> - unsigned char dfile[32];
>> + char dfile[32];
>>
>> u32 reserved1[2];
>> } __packed;
>>
>> +struct intel_dmc_header_v3 {
>> + struct intel_dmc_header_base base;
>> +
>> + /* DMC RAM start MMIO address */
>> + u32 start_mmioaddr;
>> +
>> + u32 reserved[9];
>> +
>> + /* FW filename */
>> + char dfile[32];
>> +
>> + /* Number of valid MMIO cycles present. */
>> + u32 mmio_count;
>> +
>> + /* MMIO address */
>> + u32 mmioaddr[DMC_V3_MAX_MMIO_COUNT];
>> +
>> + /* MMIO data */
>> + u32 mmiodata[DMC_V3_MAX_MMIO_COUNT];
>> +} __packed;
>> +
>> struct stepping_info {
>> char stepping;
>> char substepping;
>> @@ -333,37 +360,67 @@ static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info,
>> }
>>
>> static u32 parse_csr_fw_dmc(struct intel_csr *csr,
>> - const struct intel_dmc_header *dmc_header)
>> + const struct intel_dmc_header_base *dmc_header)
>> {
>> - unsigned int i, payload_size;
>> - u32 r;
>> + unsigned int header_len_bytes, dmc_header_size, payload_size, i;
>> + const u32 *mmioaddr, *mmiodata;
>> + u32 mmio_count, mmio_count_max;
>> u8 *payload;
>>
>> - if (sizeof(struct intel_dmc_header) != dmc_header->header_len) {
>> + BUILD_BUG_ON(ARRAY_SIZE(csr->mmioaddr) < DMC_V3_MAX_MMIO_COUNT ||
>> + ARRAY_SIZE(csr->mmioaddr) < DMC_V1_MAX_MMIO_COUNT);
>> +
>> + /* Cope with small differences between v1 and v3 */
>> + if (dmc_header->header_ver == 3) {
>
>I'm missing on this patch something like:
>
>- /* 0x01, 0x02 */
>+ /* 0x01, 0x02, 0x03 */
>near header_ver definition
>
>or maybe kill that comment at all...
nah, that is another version and it's a bit confusing. But we have
*package* header version: 1 or 2
*dmc* header version: 1 or 3
Overall structure of the firmware:
.------------.
| CSS Header |
|------------|
| Package |
| Header |
|------------|
| 20 or 32 | * 20 for package header v1
| fw_info |--.-.-. 32 for package header v2
|------------|<-' | |
| DMC header | | |
| DMC payload| | |
|------------|<---' |
| DMC header | |
| DMC payload| |
|------------| .
... .
|------------|<-----'
| DMC header |
| DMC payload|
'------------'
here the dmc_header v3 will dictate how the *DMC header*
will be parsed, that can have either 8 or 20 MMIOs to be programmed,
besides the firmware to be loaded.
I guess I could add a patch on top with this sketch as a comment to
better explain how is the layout of the file.
Lucas De Marchi
>
>The rest seems right so feel free to use
>Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
>> + const struct intel_dmc_header_v3 *v3 =
>> + (const struct intel_dmc_header_v3 *)dmc_header;
>> +
>> + mmioaddr = v3->mmioaddr;
>> + mmiodata = v3->mmiodata;
>> + mmio_count = v3->mmio_count;
>> + mmio_count_max = DMC_V3_MAX_MMIO_COUNT;
>> + /* header_len is in dwords */
>> + header_len_bytes = dmc_header->header_len * 4;
>> + dmc_header_size = sizeof(*v3);
>> + } else if (dmc_header->header_ver == 1) {
>> + const struct intel_dmc_header_v1 *v1 =
>> + (const struct intel_dmc_header_v1 *)dmc_header;
>> +
>> + mmioaddr = v1->mmioaddr;
>> + mmiodata = v1->mmiodata;
>> + mmio_count = v1->mmio_count;
>> + mmio_count_max = DMC_V1_MAX_MMIO_COUNT;
>> + header_len_bytes = dmc_header->header_len;
>> + dmc_header_size = sizeof(*v1);
>> + } else {
>> + DRM_ERROR("Unknown DMC fw header version: %u\n",
>> + dmc_header->header_ver);
>> + return 0;
>> + }
>> +
>> + if (header_len_bytes != dmc_header_size) {
>> DRM_ERROR("DMC firmware has wrong dmc header length "
>> - "(%u bytes)\n",
>> - (dmc_header->header_len));
>> + "(%u bytes)\n", header_len_bytes);
>> return 0;
>> }
>>
>> /* Cache the dmc header info. */
>> - if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
>> - DRM_ERROR("DMC firmware has wrong mmio count %u\n",
>> - dmc_header->mmio_count);
>> + if (mmio_count > mmio_count_max) {
>> + DRM_ERROR("DMC firmware has wrong mmio count %u\n", mmio_count);
>> return 0;
>> }
>>
>> - csr->mmio_count = dmc_header->mmio_count;
>> - for (i = 0; i < dmc_header->mmio_count; i++) {
>> - if (dmc_header->mmioaddr[i] < CSR_MMIO_START_RANGE ||
>> - dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
>> + for (i = 0; i < mmio_count; i++) {
>> + if (mmioaddr[i] < CSR_MMIO_START_RANGE ||
>> + mmioaddr[i] > CSR_MMIO_END_RANGE) {
>> DRM_ERROR("DMC firmware has wrong mmio address 0x%x\n",
>> - dmc_header->mmioaddr[i]);
>> + mmioaddr[i]);
>> return 0;
>> }
>> - csr->mmioaddr[i] = _MMIO(dmc_header->mmioaddr[i]);
>> - csr->mmiodata[i] = dmc_header->mmiodata[i];
>> + csr->mmioaddr[i] = _MMIO(mmioaddr[i]);
>> + csr->mmiodata[i] = mmiodata[i];
>> }
>> + csr->mmio_count = mmio_count;
>>
>> /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
>> payload_size = dmc_header->fw_size * 4;
>> @@ -379,12 +436,10 @@ static u32 parse_csr_fw_dmc(struct intel_csr *csr,
>> return 0;
>> }
>>
>> - r = sizeof(struct intel_dmc_header);
>> - payload = (u8 *)(dmc_header) + r;
>> + payload = (u8 *)(dmc_header) + header_len_bytes;
>> memcpy(csr->dmc_payload, payload, payload_size);
>> - r += payload_size;
>>
>> - return r;
>> + return header_len_bytes + payload_size;
>> }
>>
>> static u32
>> @@ -470,7 +525,7 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
>> {
>> struct intel_css_header *css_header;
>> struct intel_package_header *package_header;
>> - struct intel_dmc_header *dmc_header;
>> + struct intel_dmc_header_base *dmc_header;
>> struct intel_csr *csr = &dev_priv->csr;
>> const struct stepping_info *si = intel_get_stepping_info(dev_priv);
>> u32 readcount = 0;
>> @@ -496,7 +551,7 @@ static u32 *parse_csr_fw(struct drm_i915_private *dev_priv,
>> readcount += r;
>>
>> /* Extract dmc_header information. */
>> - dmc_header = (struct intel_dmc_header *)&fw->data[readcount];
>> + dmc_header = (struct intel_dmc_header_base *)&fw->data[readcount];
>> r = parse_csr_fw_dmc(csr, dmc_header);
>> if (!r)
>> return NULL;
>> --
>> 2.21.0
>>
More information about the Intel-gfx
mailing list