| Message ID | 20240523-edp-panel-drop-v1-1-045d62511d09@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] drm/panel-edp: add fat warning against adding new panel compatibles | expand |
On 23/05/2024 00:07, Dmitry Baryshkov wrote: > Add a fat warning against adding new panel compatibles to the panel-edp > driver. All new users of the eDP panels are supposed to use the generic > "edp-panel" compatible device on the AUX bus. The remaining compatibles > are either used by the existing DT or were used previously and are > retained for backwards compatibility. > > Suggested-by: Doug Anderson <dianders@chromium.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > The following compatibles were never used by the devices supported by > the upstream kernel and are a subject to possible removal: > > - auo,b133han05 > - auo,b140han06 > - ivo,m133nwf4-r0 > - lg,lp097qx1-spa1 > - lg,lp129qe > - samsung,lsn122dl01-c01 > - samsung,ltn140at29-301 > - sharp,ld-d5116z01b > - sharp,lq140m1jw46 > - starry,kr122ea0sra > > I'm considering dropping them, unless there is a good reason not to do > so. > --- > drivers/gpu/drm/panel/panel-edp.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > index 6db277efcbb7..95b25ec67168 100644 > --- a/drivers/gpu/drm/panel/panel-edp.c > +++ b/drivers/gpu/drm/panel/panel-edp.c > @@ -1776,7 +1776,23 @@ static const struct of_device_id platform_of_match[] = { > { > /* Must be first */ > .compatible = "edp-panel", > - }, { > + }, > + /* > + * Do not add panels to the list below unless they cannot be handled by > + * the generic edp-panel compatible. > + * > + * The only two valid reasons are: > + * - because of the panel issues (e.g. broken EDID or broken > + * identification), > + * - because the platform which uses the panel didn't wire up the AUX > + * bus properly. > + * > + * In all other cases the platform should use the aux-bus and declare > + * the panel using the 'edp-panel' compatible as a device on the AUX > + * bus. The lack of the aux-bus support is not a valid case. Platforms > + * are urged to be converted to declaring panels in a proper way. > + */ > + { > .compatible = "auo,b101ean01", > .data = &auo_b101ean01, > }, { > > --- > base-commit: 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b > change-id: 20240523-edp-panel-drop-00aa239b3c6b > > Best regards, Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Hi, On Wed, May 22, 2024 at 3:07 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > Add a fat warning against adding new panel compatibles to the panel-edp > driver. All new users of the eDP panels are supposed to use the generic > "edp-panel" compatible device on the AUX bus. The remaining compatibles > are either used by the existing DT or were used previously and are > retained for backwards compatibility. > > Suggested-by: Doug Anderson <dianders@chromium.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > The following compatibles were never used by the devices supported by > the upstream kernel and are a subject to possible removal: > > - auo,b133han05 Ack. Looks like this was added by Bjorn but by the time the dts for the board (from context, sc8180x-primus) using it made it upstream it was using "edp-panel". > - auo,b140han06 Ack. Same as above, but this time the board was "sc8180x-lenovo-flex-5g". > - ivo,m133nwf4-r0 Ack. Also added by Bjorn but not easy to tell from context which board it was from. "m133nwf4" never shows up in the history of arm64 qcom dts files. > - lg,lp097qx1-spa1 Maybe. Added by Yakir at Rockchip. I don't think this was ChromeOS related so I don't have any inside knowledge. Presumably this is for some other hardware they were using. Probably worth CCing Rockchip mailing list. It's entirely possible that they have downstream dts files using this and there's no requirement to upstream dts files that I'm aware of. > - lg,lp129qe NAK. See "arch/arm/boot/dts/nvidia/tegra124-venice2.dts" > - samsung,lsn122dl01-c01 Maybe. Also by Yakir at Rockchip and also doesn't appear to be ChromeOS, so you should ask them. > - samsung,ltn140at29-301 NAK. arch/arm/boot/dts/nvidia/tegra124-nyan-blaze.dts > - sharp,ld-d5116z01b Added by Jeffrey Hugo. Maybe Cc him to make sure it's OK to delete? > - sharp,lq140m1jw46 Ack. Feel free to delete. This was used in the sc7280 reference board (hoglin/zoglin) and by the time the dts made it upstream it was already using generic edp-panel. > - starry,kr122ea0sra Ack. From my previous notes: "starry,kr122ea0sra - rk3399-gru-gru (reference board, not upstream)". Nobody needs this. > I'm considering dropping them, unless there is a good reason not to do > so. > --- > drivers/gpu/drm/panel/panel-edp.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > index 6db277efcbb7..95b25ec67168 100644 > --- a/drivers/gpu/drm/panel/panel-edp.c > +++ b/drivers/gpu/drm/panel/panel-edp.c > @@ -1776,7 +1776,23 @@ static const struct of_device_id platform_of_match[] = { > { > /* Must be first */ > .compatible = "edp-panel", > - }, { > + }, > + /* > + * Do not add panels to the list below unless they cannot be handled by > + * the generic edp-panel compatible. > + * > + * The only two valid reasons are: > + * - because of the panel issues (e.g. broken EDID or broken > + * identification), > + * - because the platform which uses the panel didn't wire up the AUX > + * bus properly. You mean that they physically didn't wire up the AUX bus? I don't think that's actually possible. I don't believe you can use an eDP panel without this working. Conceivably a reason to add here is if there's some old platform that hasn't coded up support for AUX bus. ...but it would be much preferred to update the platform driver to support it. -Doug
On Thu, May 23, 2024 at 08:36:39AM -0700, Doug Anderson wrote: > Hi, > > On Wed, May 22, 2024 at 3:07 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > Add a fat warning against adding new panel compatibles to the panel-edp > > driver. All new users of the eDP panels are supposed to use the generic > > "edp-panel" compatible device on the AUX bus. The remaining compatibles > > are either used by the existing DT or were used previously and are > > retained for backwards compatibility. > > > > Suggested-by: Doug Anderson <dianders@chromium.org> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > The following compatibles were never used by the devices supported by > > the upstream kernel and are a subject to possible removal: > > > > - auo,b133han05 > > Ack. Looks like this was added by Bjorn but by the time the dts for > the board (from context, sc8180x-primus) using it made it upstream it > was using "edp-panel". > > > > - auo,b140han06 > > Ack. Same as above, but this time the board was "sc8180x-lenovo-flex-5g". > > > > - ivo,m133nwf4-r0 > > Ack. Also added by Bjorn but not easy to tell from context which board > it was from. "m133nwf4" never shows up in the history of arm64 qcom > dts files. > > > > - lg,lp097qx1-spa1 > > Maybe. Added by Yakir at Rockchip. I don't think this was ChromeOS > related so I don't have any inside knowledge. Presumably this is for > some other hardware they were using. Probably worth CCing Rockchip > mailing list. It's entirely possible that they have downstream dts > files using this and there's no requirement to upstream dts files that > I'm aware of. I see. Adding him to the CC list. > > > > - lg,lp129qe > > NAK. See "arch/arm/boot/dts/nvidia/tegra124-venice2.dts" Yes. I even had a comment next to it. And then blindly posted it here. > > > > - samsung,lsn122dl01-c01 > > Maybe. Also by Yakir at Rockchip and also doesn't appear to be > ChromeOS, so you should ask them. > > > > - samsung,ltn140at29-301 > > NAK. arch/arm/boot/dts/nvidia/tegra124-nyan-blaze.dts > > > > - sharp,ld-d5116z01b > > Added by Jeffrey Hugo. Maybe Cc him to make sure it's OK to delete? I pinged him on IRC. > > > > - sharp,lq140m1jw46 > > Ack. Feel free to delete. This was used in the sc7280 reference board > (hoglin/zoglin) and by the time the dts made it upstream it was > already using generic edp-panel. > > > > - starry,kr122ea0sra > > Ack. From my previous notes: "starry,kr122ea0sra - rk3399-gru-gru > (reference board, not upstream)". Nobody needs this. > > > > I'm considering dropping them, unless there is a good reason not to do > > so. > > --- > > drivers/gpu/drm/panel/panel-edp.c | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > > index 6db277efcbb7..95b25ec67168 100644 > > --- a/drivers/gpu/drm/panel/panel-edp.c > > +++ b/drivers/gpu/drm/panel/panel-edp.c > > @@ -1776,7 +1776,23 @@ static const struct of_device_id platform_of_match[] = { > > { > > /* Must be first */ > > .compatible = "edp-panel", > > - }, { > > + }, > > + /* > > + * Do not add panels to the list below unless they cannot be handled by > > + * the generic edp-panel compatible. > > + * > > + * The only two valid reasons are: > > + * - because of the panel issues (e.g. broken EDID or broken > > + * identification), > > + * - because the platform which uses the panel didn't wire up the AUX > > + * bus properly. > > You mean that they physically didn't wire up the AUX bus? I don't > think that's actually possible. I don't believe you can use an eDP > panel without this working. Conceivably a reason to add here is if > there's some old platform that hasn't coded up support for AUX bus. > ...but it would be much preferred to update the platform driver to > support it. I was thinking about the DT/driver side of the story. Let me rephrase it in a cleaner way.
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 6db277efcbb7..95b25ec67168 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -1776,7 +1776,23 @@ static const struct of_device_id platform_of_match[] = { { /* Must be first */ .compatible = "edp-panel", - }, { + }, + /* + * Do not add panels to the list below unless they cannot be handled by + * the generic edp-panel compatible. + * + * The only two valid reasons are: + * - because of the panel issues (e.g. broken EDID or broken + * identification), + * - because the platform which uses the panel didn't wire up the AUX + * bus properly. + * + * In all other cases the platform should use the aux-bus and declare + * the panel using the 'edp-panel' compatible as a device on the AUX + * bus. The lack of the aux-bus support is not a valid case. Platforms + * are urged to be converted to declaring panels in a proper way. + */ + { .compatible = "auo,b101ean01", .data = &auo_b101ean01, }, {
Add a fat warning against adding new panel compatibles to the panel-edp driver. All new users of the eDP panels are supposed to use the generic "edp-panel" compatible device on the AUX bus. The remaining compatibles are either used by the existing DT or were used previously and are retained for backwards compatibility. Suggested-by: Doug Anderson <dianders@chromium.org> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- The following compatibles were never used by the devices supported by the upstream kernel and are a subject to possible removal: - auo,b133han05 - auo,b140han06 - ivo,m133nwf4-r0 - lg,lp097qx1-spa1 - lg,lp129qe - samsung,lsn122dl01-c01 - samsung,ltn140at29-301 - sharp,ld-d5116z01b - sharp,lq140m1jw46 - starry,kr122ea0sra I'm considering dropping them, unless there is a good reason not to do so. --- drivers/gpu/drm/panel/panel-edp.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) --- base-commit: 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b change-id: 20240523-edp-panel-drop-00aa239b3c6b Best regards,