| Message ID | 20220520094600.2066945-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dsi: fix VBT send packet port selection for ICL+ | expand |
On Fri, May 20, 2022 at 12:46:00PM +0300, Jani Nikula wrote: > The VBT send packet port selection was never updated for ICL+ where the > 2nd link is on port B instead of port C as in VLV+ DSI. > > First, single link DSI needs to use the configured port instead of > relying on the VBT sequence block port. Remove the hard-coded port C > check here and make it generic. For reference, see commit f915084edc5a > ("drm/i915: Changes related to the sequence port no for") for the > original VLV specific fix. > > Second, the sequence block port number is either 0 or 1, where 1 > indicates the 2nd link. Remove the hard-coded port C here for 2nd > link. (This could be a "find second set bit" on DSI ports, but just > check the two possible options.) > > Third, sanity check the result with a warning to avoid a NULL pointer > dereference. > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5984 > Cc: stable@vger.kernel.org # v4.19+ > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 33 +++++++++++++------- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c > index f370e9c4350d..dd24aef925f2 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c > +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c > @@ -125,9 +125,25 @@ struct i2c_adapter_lookup { > #define ICL_GPIO_DDPA_CTRLCLK_2 8 > #define ICL_GPIO_DDPA_CTRLDATA_2 9 > > -static enum port intel_dsi_seq_port_to_port(u8 port) > +static enum port intel_dsi_seq_port_to_port(struct intel_dsi *intel_dsi, > + u8 seq_port) > { > - return port ? PORT_C : PORT_A; > + /* > + * If single link DSI is being used on any port, the VBT sequence block > + * send packet apparently always has 0 for the port. Just use the port > + * we have configured, and ignore the sequence block port. > + */ > + if (hweight8(intel_dsi->ports) == 1) > + return ffs(intel_dsi->ports) - 1; > + > + if (seq_port) { > + if (intel_dsi->ports & PORT_B) > + return PORT_B; > + else if (intel_dsi->ports & PORT_C) > + return PORT_C; > + } > + > + return PORT_A; Hmm. I guess a bit more generic way to express that could be to just pick the Nth set bit from intel_dsi->ports, where N==seq_port. Assuming seq_port is just an index. But I guess we're not really expecting to grow more DSI ports any time soon, so this seems sufficient for the current situation. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > } > > static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, > @@ -149,15 +165,10 @@ static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, > > seq_port = (flags >> MIPI_PORT_SHIFT) & 3; > > - /* For DSI single link on Port A & C, the seq_port value which is > - * parsed from Sequence Block#53 of VBT has been set to 0 > - * Now, read/write of packets for the DSI single link on Port A and > - * Port C will based on the DVO port from VBT block 2. > - */ > - if (intel_dsi->ports == (1 << PORT_C)) > - port = PORT_C; > - else > - port = intel_dsi_seq_port_to_port(seq_port); > + port = intel_dsi_seq_port_to_port(intel_dsi, seq_port); > + > + if (drm_WARN_ON(&dev_priv->drm, !intel_dsi->dsi_hosts[port])) > + goto out; > > dsi_device = intel_dsi->dsi_hosts[port]->device; > if (!dsi_device) { > -- > 2.30.2
On Fri, 20 May 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, May 20, 2022 at 12:46:00PM +0300, Jani Nikula wrote: >> The VBT send packet port selection was never updated for ICL+ where the >> 2nd link is on port B instead of port C as in VLV+ DSI. >> >> First, single link DSI needs to use the configured port instead of >> relying on the VBT sequence block port. Remove the hard-coded port C >> check here and make it generic. For reference, see commit f915084edc5a >> ("drm/i915: Changes related to the sequence port no for") for the >> original VLV specific fix. >> >> Second, the sequence block port number is either 0 or 1, where 1 >> indicates the 2nd link. Remove the hard-coded port C here for 2nd >> link. (This could be a "find second set bit" on DSI ports, but just >> check the two possible options.) >> >> Third, sanity check the result with a warning to avoid a NULL pointer >> dereference. >> >> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5984 >> Cc: stable@vger.kernel.org # v4.19+ >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 33 +++++++++++++------- >> 1 file changed, 22 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c >> index f370e9c4350d..dd24aef925f2 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c >> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c >> @@ -125,9 +125,25 @@ struct i2c_adapter_lookup { >> #define ICL_GPIO_DDPA_CTRLCLK_2 8 >> #define ICL_GPIO_DDPA_CTRLDATA_2 9 >> >> -static enum port intel_dsi_seq_port_to_port(u8 port) >> +static enum port intel_dsi_seq_port_to_port(struct intel_dsi *intel_dsi, >> + u8 seq_port) >> { >> - return port ? PORT_C : PORT_A; >> + /* >> + * If single link DSI is being used on any port, the VBT sequence block >> + * send packet apparently always has 0 for the port. Just use the port >> + * we have configured, and ignore the sequence block port. >> + */ >> + if (hweight8(intel_dsi->ports) == 1) >> + return ffs(intel_dsi->ports) - 1; >> + >> + if (seq_port) { >> + if (intel_dsi->ports & PORT_B) >> + return PORT_B; >> + else if (intel_dsi->ports & PORT_C) >> + return PORT_C; >> + } >> + >> + return PORT_A; > > Hmm. I guess a bit more generic way to express that could be > to just pick the Nth set bit from intel_dsi->ports, where N==seq_port. > Assuming seq_port is just an index. But I guess we're not really > expecting to grow more DSI ports any time soon, so this seems > sufficient for the current situation. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks, pushed to drm-intel-next. BR, Jani. > >> } >> >> static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, >> @@ -149,15 +165,10 @@ static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, >> >> seq_port = (flags >> MIPI_PORT_SHIFT) & 3; >> >> - /* For DSI single link on Port A & C, the seq_port value which is >> - * parsed from Sequence Block#53 of VBT has been set to 0 >> - * Now, read/write of packets for the DSI single link on Port A and >> - * Port C will based on the DVO port from VBT block 2. >> - */ >> - if (intel_dsi->ports == (1 << PORT_C)) >> - port = PORT_C; >> - else >> - port = intel_dsi_seq_port_to_port(seq_port); >> + port = intel_dsi_seq_port_to_port(intel_dsi, seq_port); >> + >> + if (drm_WARN_ON(&dev_priv->drm, !intel_dsi->dsi_hosts[port])) >> + goto out; >> >> dsi_device = intel_dsi->dsi_hosts[port]->device; >> if (!dsi_device) { >> -- >> 2.30.2
diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c index f370e9c4350d..dd24aef925f2 100644 --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c @@ -125,9 +125,25 @@ struct i2c_adapter_lookup { #define ICL_GPIO_DDPA_CTRLCLK_2 8 #define ICL_GPIO_DDPA_CTRLDATA_2 9 -static enum port intel_dsi_seq_port_to_port(u8 port) +static enum port intel_dsi_seq_port_to_port(struct intel_dsi *intel_dsi, + u8 seq_port) { - return port ? PORT_C : PORT_A; + /* + * If single link DSI is being used on any port, the VBT sequence block + * send packet apparently always has 0 for the port. Just use the port + * we have configured, and ignore the sequence block port. + */ + if (hweight8(intel_dsi->ports) == 1) + return ffs(intel_dsi->ports) - 1; + + if (seq_port) { + if (intel_dsi->ports & PORT_B) + return PORT_B; + else if (intel_dsi->ports & PORT_C) + return PORT_C; + } + + return PORT_A; } static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, @@ -149,15 +165,10 @@ static const u8 *mipi_exec_send_packet(struct intel_dsi *intel_dsi, seq_port = (flags >> MIPI_PORT_SHIFT) & 3; - /* For DSI single link on Port A & C, the seq_port value which is - * parsed from Sequence Block#53 of VBT has been set to 0 - * Now, read/write of packets for the DSI single link on Port A and - * Port C will based on the DVO port from VBT block 2. - */ - if (intel_dsi->ports == (1 << PORT_C)) - port = PORT_C; - else - port = intel_dsi_seq_port_to_port(seq_port); + port = intel_dsi_seq_port_to_port(intel_dsi, seq_port); + + if (drm_WARN_ON(&dev_priv->drm, !intel_dsi->dsi_hosts[port])) + goto out; dsi_device = intel_dsi->dsi_hosts[port]->device; if (!dsi_device) {
The VBT send packet port selection was never updated for ICL+ where the 2nd link is on port B instead of port C as in VLV+ DSI. First, single link DSI needs to use the configured port instead of relying on the VBT sequence block port. Remove the hard-coded port C check here and make it generic. For reference, see commit f915084edc5a ("drm/i915: Changes related to the sequence port no for") for the original VLV specific fix. Second, the sequence block port number is either 0 or 1, where 1 indicates the 2nd link. Remove the hard-coded port C here for 2nd link. (This could be a "find second set bit" on DSI ports, but just check the two possible options.) Third, sanity check the result with a warning to avoid a NULL pointer dereference. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5984 Cc: stable@vger.kernel.org # v4.19+ Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 33 +++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-)