| Message ID | 1507015023-16524-1-git-send-email-madhav.chauhan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 03 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com> wrote: > This patch parse DSI backlight/cabc ports info from > VBT and save them inside local strucutre. This saved info > can be directly used while initializing DSI for different > platforms instead of parsing for each platform. > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_bios.c | 63 ++++++++++++++++++++++++++++++++------- > 2 files changed, 55 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index b7cba89..fc472bb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1751,6 +1751,8 @@ struct intel_vbt_data { > u8 seq_version; > u32 size; > u8 *data; > + u16 bl_ports; > + u16 cabc_ports; This is right in the middle of the sequence data. Please move up e.g. between pps and seq_version. > const u8 *sequence[MIPI_SEQ_MAX]; > } dsi; > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 3747d8d..88a72cc 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) > dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time; > } > > +static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv, > + u16 version, enum port port) > +{ > + if (dev_priv->vbt.dsi.config->dual_link && version < 197) { > + /* > + * These fields are introduced from the VBT version 197 onwards, > + * so making sure that these bits are set zero in the previous > + * versions. > + */ > + dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; > + dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; You could remove this in patch 2. Nobody should be looking at it anymore. > + dev_priv->vbt.dsi.bl_ports = 0; > + dev_priv->vbt.dsi.cabc_ports = 0; This you don't have to do anyway, it's all zeros by default. > + return; > + } else if (dev_priv->vbt.dsi.config->dual_link) { > + switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) { > + case DL_DCS_PORT_A: > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A); > + break; > + case DL_DCS_PORT_C: > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_C); > + break; > + default: > + case DL_DCS_PORT_A_AND_C: > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | BIT(PORT_C); > + break; > + } > + > + switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) { > + case DL_DCS_PORT_A: > + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A); > + break; > + case DL_DCS_PORT_C: > + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C); > + break; > + default: > + case DL_DCS_PORT_A_AND_C: > + dev_priv->vbt.dsi.cabc_ports = > + BIT(PORT_A) | BIT(PORT_C); > + break; > + } > + } else { > + dev_priv->vbt.dsi.bl_ports = BIT(port); > + dev_priv->vbt.dsi.cabc_ports = BIT(port); > + } > + > + if (!dev_priv->vbt.dsi.config->cabc_supported) > + dev_priv->vbt.dsi.cabc_ports = 0; Would seem reasonable to not initalize it in the first place if it's not supported. If you do a series of early returns starting with the bdb version check, then !dual_link, then !cabc_supported, it might be easiest. > +} > + > static void > parse_mipi_config(struct drm_i915_private *dev_priv, > const struct bdb_header *bdb) > @@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private *dev_priv, > const struct mipi_config *config; > const struct mipi_pps_data *pps; > int panel_type = dev_priv->vbt.panel_type; > + enum port port; > > /* parse MIPI blocks only if LFP type is MIPI */ > - if (!intel_bios_is_dsi_present(dev_priv, NULL)) > + if (!intel_bios_is_dsi_present(dev_priv, &port)) > return; > > /* Initialize this to undefined indicating no generic MIPI support */ > @@ -781,15 +832,7 @@ parse_mipi_config(struct drm_i915_private *dev_priv, > return; > } > > - /* > - * These fields are introduced from the VBT version 197 onwards, > - * so making sure that these bits are set zero in the previous > - * versions. > - */ > - if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) { > - dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; > - dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; > - } > + parse_dsi_backlight_ports(dev_priv, bdb->version, port); > > /* We have mandatory mipi config blocks. Initialize as generic panel */ > dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
> -----Original Message----- > From: Nikula, Jani > Sent: Tuesday, October 10, 2017 12:47 PM > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Hiremath, Shashidhar <shashidhar.hiremath@intel.com>; Shankar, Uma > <uma.shankar@intel.com>; Chauhan, Madhav > <madhav.chauhan@intel.com> > Subject: Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports. > > On Tue, 03 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com> > wrote: > > This patch parse DSI backlight/cabc ports info from VBT and save them > > inside local strucutre. This saved info can be directly used while > > initializing DSI for different platforms instead of parsing for each > > platform. > > > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_bios.c | 63 > > ++++++++++++++++++++++++++++++++------- > > 2 files changed, 55 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h index b7cba89..fc472bb 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1751,6 +1751,8 @@ struct intel_vbt_data { > > u8 seq_version; > > u32 size; > > u8 *data; > > + u16 bl_ports; > > + u16 cabc_ports; > > This is right in the middle of the sequence data. Please move up e.g. between > pps and seq_version. Ok. > > > const u8 *sequence[MIPI_SEQ_MAX]; > > } dsi; > > > > diff --git a/drivers/gpu/drm/i915/intel_bios.c > > b/drivers/gpu/drm/i915/intel_bios.c > > index 3747d8d..88a72cc 100644 > > --- a/drivers/gpu/drm/i915/intel_bios.c > > +++ b/drivers/gpu/drm/i915/intel_bios.c > > @@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv, > const struct bdb_header *bdb) > > dev_priv->vbt.psr.tp2_tp3_wakeup_time = > > psr_table->tp2_tp3_wakeup_time; } > > > > +static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv, > > + u16 version, enum port port) { > > + if (dev_priv->vbt.dsi.config->dual_link && version < 197) { > > + /* > > + * These fields are introduced from the VBT version 197 > onwards, > > + * so making sure that these bits are set zero in the previous > > + * versions. > > + */ > > + dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; > > + dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; > > You could remove this in patch 2. Nobody should be looking at it anymore. Patch 2 means?? Next version of this patch or patch 2 of this series. Patch 2 of this series doesn't use these variables. Please clarify. > > > + dev_priv->vbt.dsi.bl_ports = 0; > > + dev_priv->vbt.dsi.cabc_ports = 0; > > This you don't have to do anyway, it's all zeros by default. Ok. > > > + return; > > + } else if (dev_priv->vbt.dsi.config->dual_link) { > > + switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) { > > + case DL_DCS_PORT_A: > > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A); > > + break; > > + case DL_DCS_PORT_C: > > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_C); > > + break; > > + default: > > + case DL_DCS_PORT_A_AND_C: > > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | > BIT(PORT_C); > > + break; > > + } > > + > > + switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) { > > + case DL_DCS_PORT_A: > > + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A); > > + break; > > + case DL_DCS_PORT_C: > > + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C); > > + break; > > + default: > > + case DL_DCS_PORT_A_AND_C: > > + dev_priv->vbt.dsi.cabc_ports = > > + BIT(PORT_A) | BIT(PORT_C); > > + break; > > + } > > + } else { > > + dev_priv->vbt.dsi.bl_ports = BIT(port); > > + dev_priv->vbt.dsi.cabc_ports = BIT(port); > > + } > > + > > + if (!dev_priv->vbt.dsi.config->cabc_supported) > > + dev_priv->vbt.dsi.cabc_ports = 0; > > Would seem reasonable to not initalize it in the first place if it's not > supported. Agree. How about putting a check condition !cabc_supported before parsing dl_dcs_cabc_ports in switch case?? >If you do a series of early returns starting with the bdb version > check, then !dual_link, then !cabc_supported, it might be easiest. But even if for !dual_link, still we need to fill newly added cabc_ports and bl_ports. Regards, Madhav > > > +} > > + > > static void > > parse_mipi_config(struct drm_i915_private *dev_priv, > > const struct bdb_header *bdb) > > @@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private > *dev_priv, > > const struct mipi_config *config; > > const struct mipi_pps_data *pps; > > int panel_type = dev_priv->vbt.panel_type; > > + enum port port; > > > > /* parse MIPI blocks only if LFP type is MIPI */ > > - if (!intel_bios_is_dsi_present(dev_priv, NULL)) > > + if (!intel_bios_is_dsi_present(dev_priv, &port)) > > return; > > > > /* Initialize this to undefined indicating no generic MIPI support > > */ @@ -781,15 +832,7 @@ parse_mipi_config(struct drm_i915_private > *dev_priv, > > return; > > } > > > > - /* > > - * These fields are introduced from the VBT version 197 onwards, > > - * so making sure that these bits are set zero in the previous > > - * versions. > > - */ > > - if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) { > > - dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; > > - dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; > > - } > > + parse_dsi_backlight_ports(dev_priv, bdb->version, port); > > > > /* We have mandatory mipi config blocks. Initialize as generic panel > */ > > dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID; > > -- > Jani Nikula, Intel Open Source Technology Center
On Tue, 10 Oct 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote: >> -----Original Message----- >> From: Nikula, Jani >> Sent: Tuesday, October 10, 2017 12:47 PM >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- >> gfx@lists.freedesktop.org >> Cc: Hiremath, Shashidhar <shashidhar.hiremath@intel.com>; Shankar, Uma >> <uma.shankar@intel.com>; Chauhan, Madhav >> <madhav.chauhan@intel.com> >> Subject: Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports. >> >> On Tue, 03 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com> >> wrote: >> > This patch parse DSI backlight/cabc ports info from VBT and save them >> > inside local strucutre. This saved info can be directly used while >> > initializing DSI for different platforms instead of parsing for each >> > platform. >> > >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> >> > --- >> > drivers/gpu/drm/i915/i915_drv.h | 2 ++ >> > drivers/gpu/drm/i915/intel_bios.c | 63 >> > ++++++++++++++++++++++++++++++++------- >> > 2 files changed, 55 insertions(+), 10 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h >> > b/drivers/gpu/drm/i915/i915_drv.h index b7cba89..fc472bb 100644 >> > --- a/drivers/gpu/drm/i915/i915_drv.h >> > +++ b/drivers/gpu/drm/i915/i915_drv.h >> > @@ -1751,6 +1751,8 @@ struct intel_vbt_data { >> > u8 seq_version; >> > u32 size; >> > u8 *data; >> > + u16 bl_ports; >> > + u16 cabc_ports; >> >> This is right in the middle of the sequence data. Please move up e.g. between >> pps and seq_version. > > Ok. > >> >> > const u8 *sequence[MIPI_SEQ_MAX]; >> > } dsi; >> > >> > diff --git a/drivers/gpu/drm/i915/intel_bios.c >> > b/drivers/gpu/drm/i915/intel_bios.c >> > index 3747d8d..88a72cc 100644 >> > --- a/drivers/gpu/drm/i915/intel_bios.c >> > +++ b/drivers/gpu/drm/i915/intel_bios.c >> > @@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv, >> const struct bdb_header *bdb) >> > dev_priv->vbt.psr.tp2_tp3_wakeup_time = >> > psr_table->tp2_tp3_wakeup_time; } >> > >> > +static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv, >> > + u16 version, enum port port) { >> > + if (dev_priv->vbt.dsi.config->dual_link && version < 197) { >> > + /* >> > + * These fields are introduced from the VBT version 197 >> onwards, >> > + * so making sure that these bits are set zero in the previous >> > + * versions. >> > + */ >> > + dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; >> > + dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; >> >> You could remove this in patch 2. Nobody should be looking at it anymore. > > Patch 2 means?? Next version of this patch or patch 2 of this series. > Patch 2 of this series doesn't use these variables. Please clarify. After patch 2 of this series, the initialization of these two fields is unnecessary, so please remove the above lines in patch 2. > >> >> > + dev_priv->vbt.dsi.bl_ports = 0; >> > + dev_priv->vbt.dsi.cabc_ports = 0; >> >> This you don't have to do anyway, it's all zeros by default. > > Ok. > >> >> > + return; >> > + } else if (dev_priv->vbt.dsi.config->dual_link) { >> > + switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) { >> > + case DL_DCS_PORT_A: >> > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A); >> > + break; >> > + case DL_DCS_PORT_C: >> > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_C); >> > + break; >> > + default: >> > + case DL_DCS_PORT_A_AND_C: >> > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | >> BIT(PORT_C); >> > + break; >> > + } >> > + >> > + switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) { >> > + case DL_DCS_PORT_A: >> > + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A); >> > + break; >> > + case DL_DCS_PORT_C: >> > + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C); >> > + break; >> > + default: >> > + case DL_DCS_PORT_A_AND_C: >> > + dev_priv->vbt.dsi.cabc_ports = >> > + BIT(PORT_A) | BIT(PORT_C); >> > + break; >> > + } >> > + } else { >> > + dev_priv->vbt.dsi.bl_ports = BIT(port); >> > + dev_priv->vbt.dsi.cabc_ports = BIT(port); >> > + } >> > + >> > + if (!dev_priv->vbt.dsi.config->cabc_supported) >> > + dev_priv->vbt.dsi.cabc_ports = 0; >> >> Would seem reasonable to not initalize it in the first place if it's not >> supported. > > Agree. How about putting a check condition !cabc_supported > before parsing dl_dcs_cabc_ports in switch case?? > >>If you do a series of early returns starting with the bdb version >> check, then !dual_link, then !cabc_supported, it might be easiest. > > But even if for !dual_link, still we need to fill newly added cabc_ports and bl_ports. > > Regards, > Madhav > >> >> > +} >> > + >> > static void >> > parse_mipi_config(struct drm_i915_private *dev_priv, >> > const struct bdb_header *bdb) >> > @@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private >> *dev_priv, >> > const struct mipi_config *config; >> > const struct mipi_pps_data *pps; >> > int panel_type = dev_priv->vbt.panel_type; >> > + enum port port; >> > >> > /* parse MIPI blocks only if LFP type is MIPI */ >> > - if (!intel_bios_is_dsi_present(dev_priv, NULL)) >> > + if (!intel_bios_is_dsi_present(dev_priv, &port)) >> > return; >> > >> > /* Initialize this to undefined indicating no generic MIPI support >> > */ @@ -781,15 +832,7 @@ parse_mipi_config(struct drm_i915_private >> *dev_priv, >> > return; >> > } >> > >> > - /* >> > - * These fields are introduced from the VBT version 197 onwards, >> > - * so making sure that these bits are set zero in the previous >> > - * versions. >> > - */ >> > - if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) { >> > - dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; >> > - dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; >> > - } >> > + parse_dsi_backlight_ports(dev_priv, bdb->version, port); >> > >> > /* We have mandatory mipi config blocks. Initialize as generic panel >> */ >> > dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID; >> >> -- >> Jani Nikula, Intel Open Source Technology Center
> -----Original Message----- > From: Nikula, Jani > Sent: Tuesday, October 10, 2017 11:27 PM > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Hiremath, Shashidhar <shashidhar.hiremath@intel.com>; Shankar, Uma > <uma.shankar@intel.com> > Subject: RE: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports. > > On Tue, 10 Oct 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com> > wrote: > >> -----Original Message----- > >> From: Nikula, Jani > >> Sent: Tuesday, October 10, 2017 12:47 PM > >> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel- > >> gfx@lists.freedesktop.org > >> Cc: Hiremath, Shashidhar <shashidhar.hiremath@intel.com>; Shankar, > >> Uma <uma.shankar@intel.com>; Chauhan, Madhav > >> <madhav.chauhan@intel.com> > >> Subject: Re: [PATCH 1/2] drm/i915: Parse DSI backlight/cabc ports. > >> > >> On Tue, 03 Oct 2017, Madhav Chauhan <madhav.chauhan@intel.com> > >> wrote: > >> > This patch parse DSI backlight/cabc ports info from VBT and save > >> > them inside local strucutre. This saved info can be directly used > >> > while initializing DSI for different platforms instead of parsing > >> > for each platform. > >> > > >> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> > >> > --- > >> > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > >> > drivers/gpu/drm/i915/intel_bios.c | 63 > >> > ++++++++++++++++++++++++++++++++------- > >> > 2 files changed, 55 insertions(+), 10 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h > >> > b/drivers/gpu/drm/i915/i915_drv.h index b7cba89..fc472bb 100644 > >> > --- a/drivers/gpu/drm/i915/i915_drv.h > >> > +++ b/drivers/gpu/drm/i915/i915_drv.h > >> > @@ -1751,6 +1751,8 @@ struct intel_vbt_data { > >> > u8 seq_version; > >> > u32 size; > >> > u8 *data; > >> > + u16 bl_ports; > >> > + u16 cabc_ports; > >> > >> This is right in the middle of the sequence data. Please move up e.g. > >> between pps and seq_version. > > > > Ok. > > > >> > >> > const u8 *sequence[MIPI_SEQ_MAX]; > >> > } dsi; > >> > > >> > diff --git a/drivers/gpu/drm/i915/intel_bios.c > >> > b/drivers/gpu/drm/i915/intel_bios.c > >> > index 3747d8d..88a72cc 100644 > >> > --- a/drivers/gpu/drm/i915/intel_bios.c > >> > +++ b/drivers/gpu/drm/i915/intel_bios.c > >> > @@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv, > >> const struct bdb_header *bdb) > >> > dev_priv->vbt.psr.tp2_tp3_wakeup_time = > >> > psr_table->tp2_tp3_wakeup_time; } > >> > > >> > +static void parse_dsi_backlight_ports(struct drm_i915_private > *dev_priv, > >> > + u16 version, enum port port) { > >> > + if (dev_priv->vbt.dsi.config->dual_link && version < 197) { > >> > + /* > >> > + * These fields are introduced from the VBT version 197 > >> onwards, > >> > + * so making sure that these bits are set zero in the previous > >> > + * versions. > >> > + */ > >> > + dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; > >> > + dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; > >> > >> You could remove this in patch 2. Nobody should be looking at it > anymore. > > > > Patch 2 means?? Next version of this patch or patch 2 of this series. > > Patch 2 of this series doesn't use these variables. Please clarify. > > After patch 2 of this series, the initialization of these two fields is > unnecessary, so please remove the above lines in patch 2. Ok. Thanks!! Regards, Madhav > > > > >> > >> > + dev_priv->vbt.dsi.bl_ports = 0; > >> > + dev_priv->vbt.dsi.cabc_ports = 0; > >> > >> This you don't have to do anyway, it's all zeros by default. > > > > Ok. > > > >> > >> > + return; > >> > + } else if (dev_priv->vbt.dsi.config->dual_link) { > >> > + switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) { > >> > + case DL_DCS_PORT_A: > >> > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A); > >> > + break; > >> > + case DL_DCS_PORT_C: > >> > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_C); > >> > + break; > >> > + default: > >> > + case DL_DCS_PORT_A_AND_C: > >> > + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | > >> BIT(PORT_C); > >> > + break; > >> > + } > >> > + > >> > + switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) { > >> > + case DL_DCS_PORT_A: > >> > + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A); > >> > + break; > >> > + case DL_DCS_PORT_C: > >> > + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C); > >> > + break; > >> > + default: > >> > + case DL_DCS_PORT_A_AND_C: > >> > + dev_priv->vbt.dsi.cabc_ports = > >> > + BIT(PORT_A) | BIT(PORT_C); > >> > + break; > >> > + } > >> > + } else { > >> > + dev_priv->vbt.dsi.bl_ports = BIT(port); > >> > + dev_priv->vbt.dsi.cabc_ports = BIT(port); > >> > + } > >> > + > >> > + if (!dev_priv->vbt.dsi.config->cabc_supported) > >> > + dev_priv->vbt.dsi.cabc_ports = 0; > >> > >> Would seem reasonable to not initalize it in the first place if it's > >> not supported. > > > > Agree. How about putting a check condition !cabc_supported before > > parsing dl_dcs_cabc_ports in switch case?? > > > >>If you do a series of early returns starting with the bdb version > >>check, then !dual_link, then !cabc_supported, it might be easiest. > > > > But even if for !dual_link, still we need to fill newly added cabc_ports and > bl_ports. > > > > Regards, > > Madhav > > > >> > >> > +} > >> > + > >> > static void > >> > parse_mipi_config(struct drm_i915_private *dev_priv, > >> > const struct bdb_header *bdb) > >> > @@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private > >> *dev_priv, > >> > const struct mipi_config *config; > >> > const struct mipi_pps_data *pps; > >> > int panel_type = dev_priv->vbt.panel_type; > >> > + enum port port; > >> > > >> > /* parse MIPI blocks only if LFP type is MIPI */ > >> > - if (!intel_bios_is_dsi_present(dev_priv, NULL)) > >> > + if (!intel_bios_is_dsi_present(dev_priv, &port)) > >> > return; > >> > > >> > /* Initialize this to undefined indicating no generic MIPI > >> > support */ @@ -781,15 +832,7 @@ parse_mipi_config(struct > >> > drm_i915_private > >> *dev_priv, > >> > return; > >> > } > >> > > >> > - /* > >> > - * These fields are introduced from the VBT version 197 onwards, > >> > - * so making sure that these bits are set zero in the previous > >> > - * versions. > >> > - */ > >> > - if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) { > >> > - dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; > >> > - dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; > >> > - } > >> > + parse_dsi_backlight_ports(dev_priv, bdb->version, port); > >> > > >> > /* We have mandatory mipi config blocks. Initialize as generic > >> > panel > >> */ > >> > dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID; > >> > >> -- > >> Jani Nikula, Intel Open Source Technology Center > > -- > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b7cba89..fc472bb 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1751,6 +1751,8 @@ struct intel_vbt_data { u8 seq_version; u32 size; u8 *data; + u16 bl_ports; + u16 cabc_ports; const u8 *sequence[MIPI_SEQ_MAX]; } dsi; diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 3747d8d..88a72cc 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -730,6 +730,56 @@ parse_psr(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) dev_priv->vbt.psr.tp2_tp3_wakeup_time = psr_table->tp2_tp3_wakeup_time; } +static void parse_dsi_backlight_ports(struct drm_i915_private *dev_priv, + u16 version, enum port port) +{ + if (dev_priv->vbt.dsi.config->dual_link && version < 197) { + /* + * These fields are introduced from the VBT version 197 onwards, + * so making sure that these bits are set zero in the previous + * versions. + */ + dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; + dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; + dev_priv->vbt.dsi.bl_ports = 0; + dev_priv->vbt.dsi.cabc_ports = 0; + return; + } else if (dev_priv->vbt.dsi.config->dual_link) { + switch (dev_priv->vbt.dsi.config->dl_dcs_backlight_ports) { + case DL_DCS_PORT_A: + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A); + break; + case DL_DCS_PORT_C: + dev_priv->vbt.dsi.bl_ports = BIT(PORT_C); + break; + default: + case DL_DCS_PORT_A_AND_C: + dev_priv->vbt.dsi.bl_ports = BIT(PORT_A) | BIT(PORT_C); + break; + } + + switch (dev_priv->vbt.dsi.config->dl_dcs_cabc_ports) { + case DL_DCS_PORT_A: + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_A); + break; + case DL_DCS_PORT_C: + dev_priv->vbt.dsi.cabc_ports = BIT(PORT_C); + break; + default: + case DL_DCS_PORT_A_AND_C: + dev_priv->vbt.dsi.cabc_ports = + BIT(PORT_A) | BIT(PORT_C); + break; + } + } else { + dev_priv->vbt.dsi.bl_ports = BIT(port); + dev_priv->vbt.dsi.cabc_ports = BIT(port); + } + + if (!dev_priv->vbt.dsi.config->cabc_supported) + dev_priv->vbt.dsi.cabc_ports = 0; +} + static void parse_mipi_config(struct drm_i915_private *dev_priv, const struct bdb_header *bdb) @@ -738,9 +788,10 @@ parse_mipi_config(struct drm_i915_private *dev_priv, const struct mipi_config *config; const struct mipi_pps_data *pps; int panel_type = dev_priv->vbt.panel_type; + enum port port; /* parse MIPI blocks only if LFP type is MIPI */ - if (!intel_bios_is_dsi_present(dev_priv, NULL)) + if (!intel_bios_is_dsi_present(dev_priv, &port)) return; /* Initialize this to undefined indicating no generic MIPI support */ @@ -781,15 +832,7 @@ parse_mipi_config(struct drm_i915_private *dev_priv, return; } - /* - * These fields are introduced from the VBT version 197 onwards, - * so making sure that these bits are set zero in the previous - * versions. - */ - if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) { - dev_priv->vbt.dsi.config->dl_dcs_cabc_ports = 0; - dev_priv->vbt.dsi.config->dl_dcs_backlight_ports = 0; - } + parse_dsi_backlight_ports(dev_priv, bdb->version, port); /* We have mandatory mipi config blocks. Initialize as generic panel */ dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
This patch parse DSI backlight/cabc ports info from VBT and save them inside local strucutre. This saved info can be directly used while initializing DSI for different platforms instead of parsing for each platform. Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/intel_bios.c | 63 ++++++++++++++++++++++++++++++++------- 2 files changed, 55 insertions(+), 10 deletions(-)