[Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
From: Pradeep Bhat pradeep.b...@intel.com
This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.
v2: Incorporated review comments from Chris Wilson
Removed intel_ as a prefix for DRRS specific declarations.
v3: Incorporated Jani's review comments
Removed function which deducts drrs mode from panel_type. Modified some
print statements. Made changes to use DRRS_NOT_SUPPORTED as 0 instead of -1.
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 13 +
drivers/gpu/drm/i915/intel_bios.c | 29 +
drivers/gpu/drm/i915/intel_bios.h | 29 +
3 files changed, 71 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..6b4d0b20 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1218,6 +1218,12 @@ struct ddi_vbt_port_info {
uint8_t supports_dp:1;
};
+enum drrs_support_type {
+ DRRS_NOT_SUPPORTED = 0,
+ STATIC_DRRS_SUPPORT = 1,
+ SEAMLESS_DRRS_SUPPORT = 2
+};
+
struct intel_vbt_data {
struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
@@ -1233,6 +1239,13 @@ struct intel_vbt_data {
int lvds_ssc_freq;
unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+ /*
+* DRRS support type (Seamless OR Static DRRS OR not supported)
+* drrs_support type Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+* These values correspond to the VBT values for drrs mode.
+*/
+ enum drrs_support_type drrs_type;
+
/* eDP */
int edp_rate;
int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 86b95ca..2414eca 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -218,6 +218,25 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
panel_type = lvds_options-panel_type;
+ dev_priv-vbt.drrs_type = (lvds_options-dps_panel_type_bits
+(panel_type * 2)) MODE_MASK;
+ /*
+* VBT has static DRRS = 0 and seamless DRRS = 2.
+* The below piece of code is required to adjust vbt.drrs_type
+* to match the enum drrs_support_type.
+*/
+ switch (dev_priv-vbt.drrs_type) {
+ case 0:
+ dev_priv-vbt.drrs_type = STATIC_DRRS_SUPPORT;
+ DRM_DEBUG_KMS(DRRS supported mode is static\n);
+ break;
+ case 2:
+ DRM_DEBUG_KMS(DRRS supported mode is seamless\n);
+ break;
+ default:
+ break;
+ }
+
lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
if (!lvds_lfp_data)
return;
@@ -516,6 +535,16 @@ parse_driver_features(struct drm_i915_private *dev_priv,
if (driver-dual_frequency)
dev_priv-render_reclock_avail = true;
+
+ DRM_DEBUG_KMS(DRRS State Enabled:%d\n, driver-drrs_enabled);
+ /*
+* If DRRS is not supported, drrs_type has to be set to 0.
+* This is because, VBT is configured in such a way that
+* static DRRS is 0 and DRRS not supported is represented by
+* driver-drrs_enabled=false
+*/
+ if (!driver-drrs_enabled)
+ dev_priv-vbt.drrs_type = driver-drrs_enabled;
}
static void
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index 282de5e..5505d6c 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -281,6 +281,9 @@ struct bdb_general_definitions {
union child_device_config devices[0];
} __packed;
+/* Mask for DRRS / Panel Channel / SSC / BLT control bits extraction */
+#define MODE_MASK 0x3
+
struct bdb_lvds_options {
u8 panel_type;
u8 rsvd1;
@@ -293,6 +296,18 @@ struct bdb_lvds_options {
u8 lvds_edid:1;
u8 rsvd2:1;
u8 rsvd4;
+ /* LVDS Panel channel bits stored here */
+ u32 lvds_panel_channel_bits;
+ /* LVDS SSC (Spread Spectrum Clock) bits stored here. */
+ u16 ssc_bits;
+ u16 ssc_freq;
+ u16 ssc_ddt;
+ /* Panel color depth defined here */
+ u16 panel_color_depth;
+ /* LVDS panel type bits stored here */
+ u32 dps_panel_type_bits;
+ /* LVDS
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Fri, 14 Feb 2014 15:32:18 +0530
Vandana Kannan vandana.kan...@intel.com wrote:
From: Pradeep Bhat pradeep.b...@intel.com
This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.
v2: Incorporated review comments from Chris Wilson
Removed intel_ as a prefix for DRRS specific declarations.
v3: Incorporated Jani's review comments
Removed function which deducts drrs mode from panel_type. Modified some
print statements. Made changes to use DRRS_NOT_SUPPORTED as 0 instead of -1.
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 13 +
drivers/gpu/drm/i915/intel_bios.c | 29 +
drivers/gpu/drm/i915/intel_bios.h | 29 +
3 files changed, 71 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 728b9c3..6b4d0b20 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1218,6 +1218,12 @@ struct ddi_vbt_port_info {
uint8_t supports_dp:1;
};
+enum drrs_support_type {
+ DRRS_NOT_SUPPORTED = 0,
+ STATIC_DRRS_SUPPORT = 1,
+ SEAMLESS_DRRS_SUPPORT = 2
+};
+
struct intel_vbt_data {
struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
@@ -1233,6 +1239,13 @@ struct intel_vbt_data {
int lvds_ssc_freq;
unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+ /*
+ * DRRS support type (Seamless OR Static DRRS OR not supported)
+ * drrs_support type Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+ * These values correspond to the VBT values for drrs mode.
+ */
+ enum drrs_support_type drrs_type;
+
/* eDP */
int edp_rate;
int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 86b95ca..2414eca 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -218,6 +218,25 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
panel_type = lvds_options-panel_type;
+ dev_priv-vbt.drrs_type = (lvds_options-dps_panel_type_bits
+ (panel_type * 2)) MODE_MASK;
+ /*
+ * VBT has static DRRS = 0 and seamless DRRS = 2.
+ * The below piece of code is required to adjust vbt.drrs_type
+ * to match the enum drrs_support_type.
+ */
+ switch (dev_priv-vbt.drrs_type) {
+ case 0:
+ dev_priv-vbt.drrs_type = STATIC_DRRS_SUPPORT;
+ DRM_DEBUG_KMS(DRRS supported mode is static\n);
+ break;
+ case 2:
+ DRM_DEBUG_KMS(DRRS supported mode is seamless\n);
+ break;
+ default:
+ break;
+ }
+
lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
if (!lvds_lfp_data)
return;
@@ -516,6 +535,16 @@ parse_driver_features(struct drm_i915_private *dev_priv,
if (driver-dual_frequency)
dev_priv-render_reclock_avail = true;
+
+ DRM_DEBUG_KMS(DRRS State Enabled:%d\n, driver-drrs_enabled);
+ /*
+ * If DRRS is not supported, drrs_type has to be set to 0.
+ * This is because, VBT is configured in such a way that
+ * static DRRS is 0 and DRRS not supported is represented by
+ * driver-drrs_enabled=false
+ */
+ if (!driver-drrs_enabled)
+ dev_priv-vbt.drrs_type = driver-drrs_enabled;
}
static void
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index 282de5e..5505d6c 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -281,6 +281,9 @@ struct bdb_general_definitions {
union child_device_config devices[0];
} __packed;
+/* Mask for DRRS / Panel Channel / SSC / BLT control bits extraction */
+#define MODE_MASK0x3
+
struct bdb_lvds_options {
u8 panel_type;
u8 rsvd1;
@@ -293,6 +296,18 @@ struct bdb_lvds_options {
u8 lvds_edid:1;
u8 rsvd2:1;
u8 rsvd4;
+ /* LVDS Panel channel bits stored here */
+ u32 lvds_panel_channel_bits;
+ /* LVDS SSC (Spread Spectrum Clock) bits stored here. */
+ u16 ssc_bits;
+ u16 ssc_freq;
+ u16 ssc_ddt;
+ /* Panel color depth defined here */
+ u16 panel_color_depth;
+ /* LVDS
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Mon, Feb 03, 2014 at 09:13:17AM +0530, Vandana Kannan wrote: Again, everywhere else in intel_bios.c we use panel_type, directly as it is in VBT, 0-based. Are you saying it's all wrong? panel_type can't be 1-based in this one instance, and 0-based in all other instances, right? This is actually the first priority to check. I have discussed with the author of the patch and i will modify this to make panel_type 0-based. Can we drag the author of the patch itself into this discussion, here on intel-gfx? Generally round-trip is much faster if we cut out as many middlemen as possible ... This is just a general upstreaming bkm. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Jan-30-2014 11:50 AM, Jani Nikula wrote:
On Thu, 30 Jan 2014, Vandana Kannan vandana.kan...@intel.com wrote:
On Jan-22-2014 6:39 PM, Jani Nikula wrote:
On Mon, 23 Dec 2013, Vandana Kannan vandana.kan...@intel.com wrote:
From: Pradeep Bhat pradeep.b...@intel.com
This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.
v2: Incorporated review comments from Chris Wilson
Removed intel_ as a prefix for DRRS specific declarations.
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |9 +
drivers/gpu/drm/i915/intel_bios.c | 23 +++
drivers/gpu/drm/i915/intel_bios.h | 29 +
3 files changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index ae2c80c..f8fd045 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1174,6 +1174,15 @@ struct intel_vbt_data {
int lvds_ssc_freq;
unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+ /**
This is not a kerneldoc comment, so drop the extra *.
Ok.
+ * DRRS mode type (Seamless OR Static DRRS)
+ * drrs_mode Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+ * These values correspond to the VBT values for drrs mode.
+ */
+ int drrs_mode;
+ /* DRRS enabled or disabled in VBT */
+ bool drrs_enabled;
Both the drrs_mode and drrs_enabled should be replaced by one enum
drrs_support_type which you introduce later. It's all self-explanatory
that way, and you don't need to explain so much.
There are 2 options in the VBT. drrs_enabled to check if DRRS is
supported, drrs_mode to show which type. It has been added here as it is
for readability.
I can understand the argument for anything defined in intel_bios.[ch],
but for the rest, including struct intel_vbt_data, readability comes
from other reasons than being equivalent with the VBT specs.
+
/* eDP */
int edp_rate;
int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index f88e507..5b04a69 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -195,6 +195,21 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
}
+/**
+ * This function returns the 2 bit information pertaining to a panel type
+ * present in a 32 bit field in VBT blocks. There are 16 panel types in
VBT
+ * each occupying 2 bits of information in some 32 bit fields of VBT
blocks.
+ */
+static int
+get_mode_by_paneltype(unsigned int word)
+{
+ /**
+ * The caller of this API should interpret the 2 bits
+ * based on VBT description for that field.
+ */
+ return (word ((panel_type - 1) * 2)) MODE_MASK;
Everywhere else in intel_bios.c panel_type is used 0-based.
VBT indexes panel type as 1,2,3. Therefore, we have to make the change
to match kernel's 0-based usage.
Again, everywhere else in intel_bios.c we use panel_type, directly as it
is in VBT, 0-based. Are you saying it's all wrong? panel_type can't be
1-based in this one instance, and 0-based in all other instances, right?
This is actually the first priority to check.
I have discussed with the author of the patch and i will modify this to
make panel_type 0-based.
+}
You use the above function only once. IMHO with all the explaining above
it's just too much of a burden to the reader. Just do this in the code.
Ok.
+
/* Try to find integrated panel data */
static void
parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -218,6 +233,11 @@ parse_lfp_panel_data(struct drm_i915_private
*dev_priv,
panel_type = lvds_options-panel_type;
+ dev_priv-vbt.drrs_mode =
+ get_mode_by_paneltype(lvds_options-dps_panel_type_bits);
+ DRM_DEBUG_KMS(DRRS supported mode is : %s\n,
^ drop the space here
Ok
+ (dev_priv-vbt.drrs_mode == 0) ? STATIC : SEAMLESS);
Why shouting?
+
lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
if (!lvds_lfp_data)
return;
@@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private
*dev_priv,
if (driver-dual_frequency)
dev_priv-render_reclock_avail = true;
+
+ dev_priv-vbt.drrs_enabled = driver-drrs_state;
+ DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-drrs_state);
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Jan-22-2014 6:39 PM, Jani Nikula wrote:
On Mon, 23 Dec 2013, Vandana Kannan vandana.kan...@intel.com wrote:
From: Pradeep Bhat pradeep.b...@intel.com
This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.
v2: Incorporated review comments from Chris Wilson
Removed intel_ as a prefix for DRRS specific declarations.
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |9 +
drivers/gpu/drm/i915/intel_bios.c | 23 +++
drivers/gpu/drm/i915/intel_bios.h | 29 +
3 files changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index ae2c80c..f8fd045 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1174,6 +1174,15 @@ struct intel_vbt_data {
int lvds_ssc_freq;
unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+/**
This is not a kerneldoc comment, so drop the extra *.
Ok.
+ * DRRS mode type (Seamless OR Static DRRS)
+ * drrs_mode Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+ * These values correspond to the VBT values for drrs mode.
+ */
+int drrs_mode;
+/* DRRS enabled or disabled in VBT */
+bool drrs_enabled;
Both the drrs_mode and drrs_enabled should be replaced by one enum
drrs_support_type which you introduce later. It's all self-explanatory
that way, and you don't need to explain so much.
There are 2 options in the VBT. drrs_enabled to check if DRRS is
supported, drrs_mode to show which type. It has been added here as it is
for readability.
+
/* eDP */
int edp_rate;
int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index f88e507..5b04a69 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -195,6 +195,21 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
}
+/**
+ * This function returns the 2 bit information pertaining to a panel type
+ * present in a 32 bit field in VBT blocks. There are 16 panel types in VBT
+ * each occupying 2 bits of information in some 32 bit fields of VBT blocks.
+ */
+static int
+get_mode_by_paneltype(unsigned int word)
+{
+/**
+ * The caller of this API should interpret the 2 bits
+ * based on VBT description for that field.
+ */
+return (word ((panel_type - 1) * 2)) MODE_MASK;
Everywhere else in intel_bios.c panel_type is used 0-based.
VBT indexes panel type as 1,2,3. Therefore, we have to make the change
to match kernel's 0-based usage.
+}
You use the above function only once. IMHO with all the explaining above
it's just too much of a burden to the reader. Just do this in the code.
Ok.
+
/* Try to find integrated panel data */
static void
parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -218,6 +233,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
panel_type = lvds_options-panel_type;
+dev_priv-vbt.drrs_mode =
+get_mode_by_paneltype(lvds_options-dps_panel_type_bits);
+DRM_DEBUG_KMS(DRRS supported mode is : %s\n,
^ drop the space here
Ok
+(dev_priv-vbt.drrs_mode == 0) ? STATIC : SEAMLESS);
Why shouting?
+
lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
if (!lvds_lfp_data)
return;
@@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
if (driver-dual_frequency)
dev_priv-render_reclock_avail = true;
+
+dev_priv-vbt.drrs_enabled = driver-drrs_state;
+DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-drrs_state);
^ and here and everywhere
Ok
}
static void
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index 81ed58c..0a3a751 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -281,6 +281,9 @@ struct bdb_general_definitions {
union child_device_config devices[0];
} __packed;
+/* Mask for DRRS / Panel Channel / SSC / BLT control bits extraction */
+#define MODE_MASK 0x3
+
struct bdb_lvds_options {
u8 panel_type;
u8 rsvd1;
@@ -293,6 +296,18 @@ struct bdb_lvds_options {
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Thu, 30 Jan 2014, Vandana Kannan vandana.kan...@intel.com wrote:
On Jan-22-2014 6:39 PM, Jani Nikula wrote:
On Mon, 23 Dec 2013, Vandana Kannan vandana.kan...@intel.com wrote:
From: Pradeep Bhat pradeep.b...@intel.com
This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.
v2: Incorporated review comments from Chris Wilson
Removed intel_ as a prefix for DRRS specific declarations.
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |9 +
drivers/gpu/drm/i915/intel_bios.c | 23 +++
drivers/gpu/drm/i915/intel_bios.h | 29 +
3 files changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index ae2c80c..f8fd045 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1174,6 +1174,15 @@ struct intel_vbt_data {
int lvds_ssc_freq;
unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+ /**
This is not a kerneldoc comment, so drop the extra *.
Ok.
+* DRRS mode type (Seamless OR Static DRRS)
+* drrs_mode Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+* These values correspond to the VBT values for drrs mode.
+*/
+ int drrs_mode;
+ /* DRRS enabled or disabled in VBT */
+ bool drrs_enabled;
Both the drrs_mode and drrs_enabled should be replaced by one enum
drrs_support_type which you introduce later. It's all self-explanatory
that way, and you don't need to explain so much.
There are 2 options in the VBT. drrs_enabled to check if DRRS is
supported, drrs_mode to show which type. It has been added here as it is
for readability.
I can understand the argument for anything defined in intel_bios.[ch],
but for the rest, including struct intel_vbt_data, readability comes
from other reasons than being equivalent with the VBT specs.
+
/* eDP */
int edp_rate;
int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index f88e507..5b04a69 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -195,6 +195,21 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
}
+/**
+ * This function returns the 2 bit information pertaining to a panel type
+ * present in a 32 bit field in VBT blocks. There are 16 panel types in VBT
+ * each occupying 2 bits of information in some 32 bit fields of VBT
blocks.
+ */
+static int
+get_mode_by_paneltype(unsigned int word)
+{
+ /**
+* The caller of this API should interpret the 2 bits
+* based on VBT description for that field.
+*/
+ return (word ((panel_type - 1) * 2)) MODE_MASK;
Everywhere else in intel_bios.c panel_type is used 0-based.
VBT indexes panel type as 1,2,3. Therefore, we have to make the change
to match kernel's 0-based usage.
Again, everywhere else in intel_bios.c we use panel_type, directly as it
is in VBT, 0-based. Are you saying it's all wrong? panel_type can't be
1-based in this one instance, and 0-based in all other instances, right?
This is actually the first priority to check.
+}
You use the above function only once. IMHO with all the explaining above
it's just too much of a burden to the reader. Just do this in the code.
Ok.
+
/* Try to find integrated panel data */
static void
parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -218,6 +233,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
panel_type = lvds_options-panel_type;
+ dev_priv-vbt.drrs_mode =
+ get_mode_by_paneltype(lvds_options-dps_panel_type_bits);
+ DRM_DEBUG_KMS(DRRS supported mode is : %s\n,
^ drop the space here
Ok
+ (dev_priv-vbt.drrs_mode == 0) ? STATIC : SEAMLESS);
Why shouting?
+
lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
if (!lvds_lfp_data)
return;
@@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
if (driver-dual_frequency)
dev_priv-render_reclock_avail = true;
+
+ dev_priv-vbt.drrs_enabled = driver-drrs_state;
+ DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-drrs_state);
^ and here and everywhere
Ok
}
static void
diff --git
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Mon, 23 Dec 2013, Vandana Kannan vandana.kan...@intel.com wrote:
From: Pradeep Bhat pradeep.b...@intel.com
This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.
v2: Incorporated review comments from Chris Wilson
Removed intel_ as a prefix for DRRS specific declarations.
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |9 +
drivers/gpu/drm/i915/intel_bios.c | 23 +++
drivers/gpu/drm/i915/intel_bios.h | 29 +
3 files changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ae2c80c..f8fd045 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1174,6 +1174,15 @@ struct intel_vbt_data {
int lvds_ssc_freq;
unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+ /**
This is not a kerneldoc comment, so drop the extra *.
+ * DRRS mode type (Seamless OR Static DRRS)
+ * drrs_mode Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+ * These values correspond to the VBT values for drrs mode.
+ */
+ int drrs_mode;
+ /* DRRS enabled or disabled in VBT */
+ bool drrs_enabled;
Both the drrs_mode and drrs_enabled should be replaced by one enum
drrs_support_type which you introduce later. It's all self-explanatory
that way, and you don't need to explain so much.
+
/* eDP */
int edp_rate;
int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index f88e507..5b04a69 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -195,6 +195,21 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
}
+/**
+ * This function returns the 2 bit information pertaining to a panel type
+ * present in a 32 bit field in VBT blocks. There are 16 panel types in VBT
+ * each occupying 2 bits of information in some 32 bit fields of VBT blocks.
+ */
+static int
+get_mode_by_paneltype(unsigned int word)
+{
+ /**
+ * The caller of this API should interpret the 2 bits
+ * based on VBT description for that field.
+ */
+ return (word ((panel_type - 1) * 2)) MODE_MASK;
Everywhere else in intel_bios.c panel_type is used 0-based.
+}
You use the above function only once. IMHO with all the explaining above
it's just too much of a burden to the reader. Just do this in the code.
+
/* Try to find integrated panel data */
static void
parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -218,6 +233,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
panel_type = lvds_options-panel_type;
+ dev_priv-vbt.drrs_mode =
+ get_mode_by_paneltype(lvds_options-dps_panel_type_bits);
+ DRM_DEBUG_KMS(DRRS supported mode is : %s\n,
^ drop the space here
+ (dev_priv-vbt.drrs_mode == 0) ? STATIC : SEAMLESS);
Why shouting?
+
lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
if (!lvds_lfp_data)
return;
@@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
if (driver-dual_frequency)
dev_priv-render_reclock_avail = true;
+
+ dev_priv-vbt.drrs_enabled = driver-drrs_state;
+ DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-drrs_state);
^ and here and everywhere
}
static void
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index 81ed58c..0a3a751 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -281,6 +281,9 @@ struct bdb_general_definitions {
union child_device_config devices[0];
} __packed;
+/* Mask for DRRS / Panel Channel / SSC / BLT control bits extraction */
+#define MODE_MASK0x3
+
struct bdb_lvds_options {
u8 panel_type;
u8 rsvd1;
@@ -293,6 +296,18 @@ struct bdb_lvds_options {
u8 lvds_edid:1;
u8 rsvd2:1;
u8 rsvd4;
+ /* LVDS Panel channel bits stored here */
+ u32 lvds_panel_channel_bits;
Why does this have lvds but the rest not? Why do some fields end in
bits but some others not? Some consistency please.
+ /* LVDS SSC (Spread Spectrum Clock) bits
[Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
From: Pradeep Bhat pradeep.b...@intel.com
This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.
v2: Incorporated review comments from Chris Wilson
Removed intel_ as a prefix for DRRS specific declarations.
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |9 +
drivers/gpu/drm/i915/intel_bios.c | 23 +++
drivers/gpu/drm/i915/intel_bios.h | 29 +
3 files changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ae2c80c..f8fd045 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1174,6 +1174,15 @@ struct intel_vbt_data {
int lvds_ssc_freq;
unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+ /**
+* DRRS mode type (Seamless OR Static DRRS)
+* drrs_mode Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+* These values correspond to the VBT values for drrs mode.
+*/
+ int drrs_mode;
+ /* DRRS enabled or disabled in VBT */
+ bool drrs_enabled;
+
/* eDP */
int edp_rate;
int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index f88e507..5b04a69 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -195,6 +195,21 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
}
+/**
+ * This function returns the 2 bit information pertaining to a panel type
+ * present in a 32 bit field in VBT blocks. There are 16 panel types in VBT
+ * each occupying 2 bits of information in some 32 bit fields of VBT blocks.
+ */
+static int
+get_mode_by_paneltype(unsigned int word)
+{
+ /**
+* The caller of this API should interpret the 2 bits
+* based on VBT description for that field.
+*/
+ return (word ((panel_type - 1) * 2)) MODE_MASK;
+}
+
/* Try to find integrated panel data */
static void
parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -218,6 +233,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
panel_type = lvds_options-panel_type;
+ dev_priv-vbt.drrs_mode =
+ get_mode_by_paneltype(lvds_options-dps_panel_type_bits);
+ DRM_DEBUG_KMS(DRRS supported mode is : %s\n,
+ (dev_priv-vbt.drrs_mode == 0) ? STATIC : SEAMLESS);
+
lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
if (!lvds_lfp_data)
return;
@@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
if (driver-dual_frequency)
dev_priv-render_reclock_avail = true;
+
+ dev_priv-vbt.drrs_enabled = driver-drrs_state;
+ DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-drrs_state);
}
static void
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index 81ed58c..0a3a751 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -281,6 +281,9 @@ struct bdb_general_definitions {
union child_device_config devices[0];
} __packed;
+/* Mask for DRRS / Panel Channel / SSC / BLT control bits extraction */
+#define MODE_MASK 0x3
+
struct bdb_lvds_options {
u8 panel_type;
u8 rsvd1;
@@ -293,6 +296,18 @@ struct bdb_lvds_options {
u8 lvds_edid:1;
u8 rsvd2:1;
u8 rsvd4;
+ /* LVDS Panel channel bits stored here */
+ u32 lvds_panel_channel_bits;
+ /* LVDS SSC (Spread Spectrum Clock) bits stored here. */
+ u16 ssc_bits;
+ u16 ssc_freq;
+ u16 ssc_ddt;
+ /* Panel color depth defined here */
+ u16 panel_color_depth;
+ /* LVDS panel type bits stored here */
+ u32 dps_panel_type_bits;
+ /* LVDS backlight control type bits stored here */
+ u32 blt_control_type_bits;
} __packed;
/* LFP pointer table contains entries to the struct below */
@@ -462,6 +477,20 @@ struct bdb_driver_features {
u8 hdmi_termination;
u8 custom_vbt_version;
+ /* Driver features data block */
+ u16 rmpm_state:1;
+ u16 s2ddt_state:1;
+ u16 dpst_state:1;
+ u16 bltclt_state:1;
+ u16 adb_state:1;
+ u16 drrs_state:1;
+ u16 grs_state:1;
+ u16 gpmt_state:1;
+ u16 tbt_state:1;
+ u16 psr_state:1;
+
[Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
From: Pradeep Bhat pradeep.b...@intel.com
This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.
v2: Incorporated review comments from Chris Wilson
Removed intel_ as a prefix for DRRS specific declarations.
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_drv.h |9 +
drivers/gpu/drm/i915/intel_bios.c | 23 +++
drivers/gpu/drm/i915/intel_bios.h | 29 +
3 files changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 64ed8f4..4d6665b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1175,6 +1175,15 @@ struct intel_vbt_data {
int lvds_ssc_freq;
unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+ /**
+* DRRS mode type (Seamless OR Static DRRS)
+* drrs_mode Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+* These values correspond to the VBT values for drrs mode.
+*/
+ int drrs_mode;
+ /* DRRS enabled or disabled in VBT */
+ bool drrs_enabled;
+
/* eDP */
int edp_rate;
int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 6dd622d..30ab5a3 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -195,6 +195,21 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
}
+/**
+ * This function returns the 2 bit information pertaining to a panel type
+ * present in a 32 bit field in VBT blocks. There are 16 panel types in VBT
+ * each occupying 2 bits of information in some 32 bit fields of VBT blocks.
+ */
+static int
+get_mode_by_paneltype(unsigned int word)
+{
+ /**
+* The caller of this API should interpret the 2 bits
+* based on VBT description for that field.
+*/
+ return (word ((panel_type - 1) * 2)) MODE_MASK;
+}
+
/* Try to find integrated panel data */
static void
parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -218,6 +233,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
panel_type = lvds_options-panel_type;
+ dev_priv-vbt.drrs_mode =
+ get_mode_by_paneltype(lvds_options-dps_panel_type_bits);
+ DRM_DEBUG_KMS(DRRS supported mode is : %s\n,
+ (dev_priv-vbt.drrs_mode == 0) ? STATIC : SEAMLESS);
+
lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
if (!lvds_lfp_data)
return;
@@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
if (driver-dual_frequency)
dev_priv-render_reclock_avail = true;
+
+ dev_priv-vbt.drrs_enabled = driver-drrs_state;
+ DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-drrs_state);
}
static void
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index f580a2b..8e594f6 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -202,6 +202,9 @@ struct bdb_general_features {
#define DEVICE_PORT_DVOB 0x01
#define DEVICE_PORT_DVOC 0x02
+/* Mask for DRRS / Panel Channel / SSC / BLT control bits extraction */
+#define MODE_MASK 0x3
+
/* We used to keep this struct but without any version control. We should avoid
* using it in the future, but it should be safe to keep using it in the old
* code. */
@@ -293,6 +296,18 @@ struct bdb_lvds_options {
u8 lvds_edid:1;
u8 rsvd2:1;
u8 rsvd4;
+ /* LVDS Panel channel bits stored here */
+ u32 lvds_panel_channel_bits;
+ /* LVDS SSC (Spread Spectrum Clock) bits stored here. */
+ u16 ssc_bits;
+ u16 ssc_freq;
+ u16 ssc_ddt;
+ /* Panel color depth defined here */
+ u16 panel_color_depth;
+ /* LVDS panel type bits stored here */
+ u32 dps_panel_type_bits;
+ /* LVDS backlight control type bits stored here */
+ u32 blt_control_type_bits;
} __attribute__((packed));
/* LFP pointer table contains entries to the struct below */
@@ -462,6 +477,20 @@ struct bdb_driver_features {
u8 hdmi_termination;
u8 custom_vbt_version;
+ /* Driver features data block */
+ u16 rmpm_state:1;
+ u16 s2ddt_state:1;
+ u16 dpst_state:1;
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Dec-17-2013 5:56 PM, Chris Wilson wrote: On Tue, Dec 17, 2013 at 10:58:23AM +0530, Vandana Kannan wrote: From: Pradeep Bhat pradeep.b...@intel.com This patch reads the DRRS support and Mode type from VBT fields. The read information will be stored in VBT struct during BIOS parsing. The above functionality is needed for decision making whether DRRS feature is supported in i915 driver for eDP panels. This information helps us decide if seamless DRRS can be done at runtime to support certain power saving features. This patch was tested by setting necessary bit in VBT struct and merging the new VBT with system BIOS so that we can read the value. What's the reason for the inconsistent intel_ tautology? If you are referring to the names of members under bdb_driver_features, which start with intel_, then this is something which can be modified to remove the intel_. Is it ok? @@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv, if (driver-dual_frequency) dev_priv-render_reclock_avail = true; + +dev_priv-vbt.intel_drrs_enabled = driver-intel_drrs_state; +DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-intel_drrs_state); Now this oddity needs a big explanation. Writing that will hopefully reveal a better strategy. -Chris Panel vendors specify panel capabilities via the VBT. Following this, the panel's capability to support seamless DRRS has to be read from the VBT. The same is being done in the piece of code above. Without this we cannot assume that the panel supports seamless DRRS. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Wed, Dec 18, 2013 at 01:38:44PM +0530, Vandana Kannan wrote: On Dec-17-2013 5:56 PM, Chris Wilson wrote: On Tue, Dec 17, 2013 at 10:58:23AM +0530, Vandana Kannan wrote: From: Pradeep Bhat pradeep.b...@intel.com This patch reads the DRRS support and Mode type from VBT fields. The read information will be stored in VBT struct during BIOS parsing. The above functionality is needed for decision making whether DRRS feature is supported in i915 driver for eDP panels. This information helps us decide if seamless DRRS can be done at runtime to support certain power saving features. This patch was tested by setting necessary bit in VBT struct and merging the new VBT with system BIOS so that we can read the value. What's the reason for the inconsistent intel_ tautology? If you are referring to the names of members under bdb_driver_features, which start with intel_, then this is something which can be modified to remove the intel_. Is it ok? Yes, we use intel_ as a function prefix for generic routines that apply to almost all display engines. We expect that all of our hardware specific structure are used for Intel and so don't need reminding, least of all, inconsistently. And s/drrs_state/drrs/. @@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv, if (driver-dual_frequency) dev_priv-render_reclock_avail = true; + + dev_priv-vbt.intel_drrs_enabled = driver-intel_drrs_state; + DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-intel_drrs_state); Now this oddity needs a big explanation. Writing that will hopefully reveal a better strategy. -Chris Panel vendors specify panel capabilities via the VBT. Following this, the panel's capability to support seamless DRRS has to be read from the VBT. The same is being done in the piece of code above. Without this we cannot assume that the panel supports seamless DRRS. Nevermind, I misread driver- as dev_priv-. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Dec-18-2013 2:41 PM, Chris Wilson wrote: On Wed, Dec 18, 2013 at 01:38:44PM +0530, Vandana Kannan wrote: On Dec-17-2013 5:56 PM, Chris Wilson wrote: On Tue, Dec 17, 2013 at 10:58:23AM +0530, Vandana Kannan wrote: From: Pradeep Bhat pradeep.b...@intel.com This patch reads the DRRS support and Mode type from VBT fields. The read information will be stored in VBT struct during BIOS parsing. The above functionality is needed for decision making whether DRRS feature is supported in i915 driver for eDP panels. This information helps us decide if seamless DRRS can be done at runtime to support certain power saving features. This patch was tested by setting necessary bit in VBT struct and merging the new VBT with system BIOS so that we can read the value. What's the reason for the inconsistent intel_ tautology? If you are referring to the names of members under bdb_driver_features, which start with intel_, then this is something which can be modified to remove the intel_. Is it ok? Yes, we use intel_ as a function prefix for generic routines that apply to almost all display engines. We expect that all of our hardware specific structure are used for Intel and so don't need reminding, least of all, inconsistently. And s/drrs_state/drrs/. I will make these changes. - Vandana @@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv, if (driver-dual_frequency) dev_priv-render_reclock_avail = true; + + dev_priv-vbt.intel_drrs_enabled = driver-intel_drrs_state; + DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-intel_drrs_state); Now this oddity needs a big explanation. Writing that will hopefully reveal a better strategy. -Chris Panel vendors specify panel capabilities via the VBT. Following this, the panel's capability to support seamless DRRS has to be read from the VBT. The same is being done in the piece of code above. Without this we cannot assume that the panel supports seamless DRRS. Nevermind, I misread driver- as dev_priv-. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
On Tue, Dec 17, 2013 at 10:58:23AM +0530, Vandana Kannan wrote: From: Pradeep Bhat pradeep.b...@intel.com This patch reads the DRRS support and Mode type from VBT fields. The read information will be stored in VBT struct during BIOS parsing. The above functionality is needed for decision making whether DRRS feature is supported in i915 driver for eDP panels. This information helps us decide if seamless DRRS can be done at runtime to support certain power saving features. This patch was tested by setting necessary bit in VBT struct and merging the new VBT with system BIOS so that we can read the value. What's the reason for the inconsistent intel_ tautology? @@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv, if (driver-dual_frequency) dev_priv-render_reclock_avail = true; + + dev_priv-vbt.intel_drrs_enabled = driver-intel_drrs_state; + DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-intel_drrs_state); Now this oddity needs a big explanation. Writing that will hopefully reveal a better strategy. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature
From: Pradeep Bhat pradeep.b...@intel.com
This patch reads the DRRS support and Mode type from VBT fields.
The read information will be stored in VBT struct during BIOS
parsing. The above functionality is needed for decision making
whether DRRS feature is supported in i915 driver for eDP panels.
This information helps us decide if seamless DRRS can be done
at runtime to support certain power saving features. This patch
was tested by setting necessary bit in VBT struct and merging
the new VBT with system BIOS so that we can read the value.
Signed-off-by: Pradeep Bhat pradeep.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h |9 +
drivers/gpu/drm/i915/intel_bios.c | 23 +++
drivers/gpu/drm/i915/intel_bios.h | 29 +
3 files changed, 61 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 64ed8f4..02e11dc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1175,6 +1175,15 @@ struct intel_vbt_data {
int lvds_ssc_freq;
unsigned int bios_lvds_val; /* initial [PCH_]LVDS reg val in VBIOS */
+ /**
+* DRRS mode type (Seamless OR Static DRRS)
+* drrs_mode Val 0x2 is Seamless DRRS and 0 is Static DRRS.
+* These values correspond to the VBT values for drrs mode.
+*/
+ int drrs_mode;
+ /* DRRS enabled or disabled in VBT */
+ bool intel_drrs_enabled;
+
/* eDP */
int edp_rate;
int edp_lanes;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 6dd622d..15ee0b8 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -195,6 +195,21 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
}
+/**
+ * This function returns the 2 bit information pertaining to a panel type
+ * present in a 32 bit field in VBT blocks. There are 16 panel types in VBT
+ * each occupying 2 bits of information in some 32 bit fields of VBT blocks.
+ */
+static int
+get_mode_by_paneltype(unsigned int word)
+{
+ /**
+* The caller of this API should interpret the 2 bits
+* based on VBT description for that field.
+*/
+ return (word ((panel_type - 1) * 2)) MODE_MASK;
+}
+
/* Try to find integrated panel data */
static void
parse_lfp_panel_data(struct drm_i915_private *dev_priv,
@@ -218,6 +233,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
panel_type = lvds_options-panel_type;
+ dev_priv-vbt.drrs_mode =
+ get_mode_by_paneltype(lvds_options-dps_panel_type_bits);
+ DRM_DEBUG_KMS(DRRS supported mode is : %s\n,
+ (dev_priv-vbt.drrs_mode == 0) ? STATIC : SEAMLESS);
+
lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
if (!lvds_lfp_data)
return;
@@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv,
if (driver-dual_frequency)
dev_priv-render_reclock_avail = true;
+
+ dev_priv-vbt.intel_drrs_enabled = driver-intel_drrs_state;
+ DRM_DEBUG_KMS(DRRS State Enabled : %d\n, driver-intel_drrs_state);
}
static void
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index f580a2b..da6685b 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -202,6 +202,9 @@ struct bdb_general_features {
#define DEVICE_PORT_DVOB 0x01
#define DEVICE_PORT_DVOC 0x02
+/* Mask for DRRS / Panel Channel / SSC / BLT control bits extraction */
+#define MODE_MASK 0x3
+
/* We used to keep this struct but without any version control. We should avoid
* using it in the future, but it should be safe to keep using it in the old
* code. */
@@ -293,6 +296,18 @@ struct bdb_lvds_options {
u8 lvds_edid:1;
u8 rsvd2:1;
u8 rsvd4;
+ /* LVDS Panel channel bits stored here */
+ u32 lvds_panel_channel_bits;
+ /* LVDS SSC (Spread Spectrum Clock) bits stored here. */
+ u16 ssc_bits;
+ u16 ssc_freq;
+ u16 ssc_ddt;
+ /* Panel color depth defined here */
+ u16 panel_color_depth;
+ /* LVDS panel type bits stored here */
+ u32 dps_panel_type_bits;
+ /* LVDS backlight control type bits stored here */
+ u32 blt_control_type_bits;
} __attribute__((packed));
/* LFP pointer table contains entries to the struct below */
@@ -462,6 +477,20 @@ struct bdb_driver_features {
u8 hdmi_termination;
u8 custom_vbt_version;
+ /* Driver features data block */
+ u16 intel_rmpm_state:1;
+ u16 intel_s2ddt_state:1;
+ u16 intel_dpst_state:1;
+ u16 intel_bltclt_state:1;
+ u16 intel_adb_state:1;
+ u16 intel_drrs_state:1;
+ u16
