| Message ID | 20240730195012.2595980-1-quic_abhinavk@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/msm/dp: enable widebus on all relevant chipsets | expand |
Hi Abhinav, On Tue, 30 Jul 2024 at 22:50, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > Hardware document indicates that widebus is recommended on DP on all > MDSS chipsets starting version 5.x.x and above. > > Follow the guideline and mark widebus support on all relevant > chipsets for DP. > > Fixes: 766f705204a0 ("drm/msm/dp: Remove now unused connector_type from desc") > Fixes: 1b2d98bdd7b7 ("drm/msm/dp: Add DisplayPort controller for SM8650") The issues are present in the following commits. Please consider using them instead: Fixes: 757a2f36ab09 ("drm/msm/dp: enable widebus feature for display port") Fixes: 1b2d98bdd7b7 ("drm/msm/dp: Add DisplayPort controller for SM8650") > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/dp/dp_display.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-)
On Tue, Jul 30, 2024 at 11:58:19PM +0300, Dmitry Baryshkov wrote: > Hi Abhinav, > > On Tue, 30 Jul 2024 at 22:50, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > Hardware document indicates that widebus is recommended on DP on all > > MDSS chipsets starting version 5.x.x and above. > > > > Follow the guideline and mark widebus support on all relevant > > chipsets for DP. > > > > Fixes: 766f705204a0 ("drm/msm/dp: Remove now unused connector_type from desc") > > Fixes: 1b2d98bdd7b7 ("drm/msm/dp: Add DisplayPort controller for SM8650") > > The issues are present in the following commits. Please consider using > them instead: > > Fixes: 757a2f36ab09 ("drm/msm/dp: enable widebus feature for display port") > Fixes: 1b2d98bdd7b7 ("drm/msm/dp: Add DisplayPort controller for SM8650") > But are we really fixing any bugs/issues here? While the docs do recommend widebus, we're effectively enabling more harware/features. Unless there's a strong reason (which I'm not confident that the commit message entails), I think we should drop the fixes-tags and just bring this to 6.12... Regards, Bjorn > > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/gpu/drm/msm/dp/dp_display.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > -- > With best wishes > Dmitry
On Wed, 31 Jul 2024 at 01:27, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: > > On Tue, Jul 30, 2024 at 11:58:19PM +0300, Dmitry Baryshkov wrote: > > Hi Abhinav, > > > > On Tue, 30 Jul 2024 at 22:50, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > > > Hardware document indicates that widebus is recommended on DP on all > > > MDSS chipsets starting version 5.x.x and above. > > > > > > Follow the guideline and mark widebus support on all relevant > > > chipsets for DP. > > > > > > Fixes: 766f705204a0 ("drm/msm/dp: Remove now unused connector_type from desc") > > > Fixes: 1b2d98bdd7b7 ("drm/msm/dp: Add DisplayPort controller for SM8650") > > > > The issues are present in the following commits. Please consider using > > them instead: > > > > Fixes: 757a2f36ab09 ("drm/msm/dp: enable widebus feature for display port") > > Fixes: 1b2d98bdd7b7 ("drm/msm/dp: Add DisplayPort controller for SM8650") > > > > But are we really fixing any bugs/issues here? While the docs do > recommend widebus, we're effectively enabling more harware/features. > > Unless there's a strong reason (which I'm not confident that the commit > message entails), I think we should drop the fixes-tags and just bring > this to 6.12... I'm fine either way. I'll check tomorrow if this is required to fix https://gitlab.freedesktop.org/drm/msm/-/issues/43.
On 7/30/2024 4:34 PM, Dmitry Baryshkov wrote: > On Wed, 31 Jul 2024 at 01:27, Bjorn Andersson <quic_bjorande@quicinc.com> wrote: >> >> On Tue, Jul 30, 2024 at 11:58:19PM +0300, Dmitry Baryshkov wrote: >>> Hi Abhinav, >>> >>> On Tue, 30 Jul 2024 at 22:50, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >>>> >>>> Hardware document indicates that widebus is recommended on DP on all >>>> MDSS chipsets starting version 5.x.x and above. >>>> >>>> Follow the guideline and mark widebus support on all relevant >>>> chipsets for DP. >>>> >>>> Fixes: 766f705204a0 ("drm/msm/dp: Remove now unused connector_type from desc") >>>> Fixes: 1b2d98bdd7b7 ("drm/msm/dp: Add DisplayPort controller for SM8650") >>> >>> The issues are present in the following commits. Please consider using >>> them instead: >>> >>> Fixes: 757a2f36ab09 ("drm/msm/dp: enable widebus feature for display port") >>> Fixes: 1b2d98bdd7b7 ("drm/msm/dp: Add DisplayPort controller for SM8650") >>> >> >> But are we really fixing any bugs/issues here? While the docs do >> recommend widebus, we're effectively enabling more harware/features. >> >> Unless there's a strong reason (which I'm not confident that the commit >> message entails), I think we should drop the fixes-tags and just bring >> this to 6.12... > > I'm fine either way. I'll check tomorrow if this is required to fix > https://gitlab.freedesktop.org/drm/msm/-/issues/43. > In v1, I also did not have the Fixes tag and I admit, originally even though I made the change in an attempt to fix another issue, it was not resolving the issue completely so there is no functional issue getting fixed due to the change. I have confirmed that on my setup this patch is not needed to fix that bug. I decided to add the Fixes tag in v2 because not following the hw doc recommendation is also a bug as it can lead to unexplained issues. Thats why I am still divided on this. If the consensus is not to take it as a fix, I am fine with it and will not include it and while we are compiling the changes for 6.12 this can be included without the Fixes tags. > >
On Tue, 30 Jul 2024 12:50:11 -0700, Abhinav Kumar wrote: > Hardware document indicates that widebus is recommended on DP on all > MDSS chipsets starting version 5.x.x and above. > > Follow the guideline and mark widebus support on all relevant > chipsets for DP. > > > [...] Applied, thanks! [1/1] drm/msm/dp: enable widebus on all relevant chipsets https://gitlab.freedesktop.org/lumag/msm/-/commit/c7c412202623 Best regards,
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index aead7d27305d..bbba016ebff7 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -119,7 +119,7 @@ struct msm_dp_desc { }; static const struct msm_dp_desc sc7180_dp_descs[] = { - { .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0 }, + { .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true }, {} }; @@ -130,9 +130,9 @@ static const struct msm_dp_desc sc7280_dp_descs[] = { }; static const struct msm_dp_desc sc8180x_dp_descs[] = { - { .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0 }, - { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1 }, - { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2 }, + { .io_start = 0x0ae90000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true }, + { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .wide_bus_supported = true }, + { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .wide_bus_supported = true }, {} }; @@ -149,7 +149,7 @@ static const struct msm_dp_desc sc8280xp_dp_descs[] = { }; static const struct msm_dp_desc sm8650_dp_descs[] = { - { .io_start = 0x0af54000, .id = MSM_DP_CONTROLLER_0 }, + { .io_start = 0x0af54000, .id = MSM_DP_CONTROLLER_0, .wide_bus_supported = true }, {} };
Hardware document indicates that widebus is recommended on DP on all MDSS chipsets starting version 5.x.x and above. Follow the guideline and mark widebus support on all relevant chipsets for DP. Fixes: 766f705204a0 ("drm/msm/dp: Remove now unused connector_type from desc") Fixes: 1b2d98bdd7b7 ("drm/msm/dp: Add DisplayPort controller for SM8650") Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/dp/dp_display.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)