| Message ID | 20240531-edp-panel-drop-v3-0-4c98b2b95e3a@linaro.org |
|---|---|
| Headers | show |
| Series | drm/panel-edp: remove several legacy compatibles used by the driver | expand |
Hi, On Thu, May 30, 2024 at 4:13 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > There are two ways to describe an eDP panel in device tree. The > recommended way is to add a device on the AUX bus, ideally using the > edp-panel compatible. The legacy way is to define a top-level platform > device for the panel. > > Document that adding support for eDP panels in a legacy way is strongly > discouraged (if not forbidden at all). > > While we are at it, also drop legacy compatible strings and bindings for > five panels. These compatible strings were never used by a DT file > present in Linux kernel and most likely were never used with the > upstream Linux kernel. > > The following compatibles were never used by the devices supported by > the upstream kernel and are a subject to possible removal: > > - lg,lp097qx1-spa1 > - samsung,lsn122dl01-c01 > - sharp,ld-d5116z01b > > I'm considering dropping them later, unless there is a good reason not > to do so. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > Changes in v3: > - Rephrased the warning comment, following some of Doug's suggestions. > - Link to v2: https://lore.kernel.org/r/20240529-edp-panel-drop-v2-0-fcfc457fc8dd@linaro.org > > Changes in v2: > - Actually drop support for five panels acked by Doug Andersson > - Link to v1: https://lore.kernel.org/r/20240523-edp-panel-drop-v1-1-045d62511d09@linaro.org > > --- > Dmitry Baryshkov (3): > drm/panel-edp: add fat warning against adding new panel compatibles > dt-bindings: display: panel-simple: drop several eDP panels > drm/panel-edp: drop several legacy panels > > .../bindings/display/panel/panel-simple.yaml | 10 -- > drivers/gpu/drm/panel/panel-edp.c | 192 +++------------------ > 2 files changed, 25 insertions(+), 177 deletions(-) Thanks! I'm happy to apply this series or also happy if some other drm-misc committer member wants to apply it. Probably we should wait for a DT person to make sure that they don't have any problems with the bindings change. -Doug
On 5/30/2024 5:12 PM, Dmitry Baryshkov wrote: > There are two ways to describe an eDP panel in device tree. The > recommended way is to add a device on the AUX bus, ideally using the > edp-panel compatible. The legacy way is to define a top-level platform > device for the panel. > > Document that adding support for eDP panels in a legacy way is strongly > discouraged (if not forbidden at all). > > While we are at it, also drop legacy compatible strings and bindings for > five panels. These compatible strings were never used by a DT file > present in Linux kernel and most likely were never used with the > upstream Linux kernel. > > The following compatibles were never used by the devices supported by > the upstream kernel and are a subject to possible removal: > > - lg,lp097qx1-spa1 > - samsung,lsn122dl01-c01 > - sharp,ld-d5116z01b Ok to drop the sharp one I added. It should be able to be handled by the (newish) edp-panel, but I think the TI bridge driver needs some work for the specific platform (no I2C connection) to verify. -Jeff
Hi, On Fri, May 31, 2024 at 9:18 AM Jeffrey Hugo <quic_jhugo@quicinc.com> wrote: > > On 5/30/2024 5:12 PM, Dmitry Baryshkov wrote: > > There are two ways to describe an eDP panel in device tree. The > > recommended way is to add a device on the AUX bus, ideally using the > > edp-panel compatible. The legacy way is to define a top-level platform > > device for the panel. > > > > Document that adding support for eDP panels in a legacy way is strongly > > discouraged (if not forbidden at all). > > > > While we are at it, also drop legacy compatible strings and bindings for > > five panels. These compatible strings were never used by a DT file > > present in Linux kernel and most likely were never used with the > > upstream Linux kernel. > > > > The following compatibles were never used by the devices supported by > > the upstream kernel and are a subject to possible removal: > > > > - lg,lp097qx1-spa1 > > - samsung,lsn122dl01-c01 > > - sharp,ld-d5116z01b > > Ok to drop the sharp one I added. It should be able to be handled by > the (newish) edp-panel, but I think the TI bridge driver needs some work > for the specific platform (no I2C connection) to verify. Is the platform supported upstream? If so, which platform is it? Is the TI bridge chip the ti-sn65dsi86? If so, I'm confused how you could use that bridge chip without an i2c connection, but perhaps I'm misunderstanding. :-P Thanks! -Doug
On 5/31/2024 10:20 AM, Doug Anderson wrote: > Hi, > > On Fri, May 31, 2024 at 9:18 AM Jeffrey Hugo <quic_jhugo@quicinc.com> wrote: >> >> On 5/30/2024 5:12 PM, Dmitry Baryshkov wrote: >>> There are two ways to describe an eDP panel in device tree. The >>> recommended way is to add a device on the AUX bus, ideally using the >>> edp-panel compatible. The legacy way is to define a top-level platform >>> device for the panel. >>> >>> Document that adding support for eDP panels in a legacy way is strongly >>> discouraged (if not forbidden at all). >>> >>> While we are at it, also drop legacy compatible strings and bindings for >>> five panels. These compatible strings were never used by a DT file >>> present in Linux kernel and most likely were never used with the >>> upstream Linux kernel. >>> >>> The following compatibles were never used by the devices supported by >>> the upstream kernel and are a subject to possible removal: >>> >>> - lg,lp097qx1-spa1 >>> - samsung,lsn122dl01-c01 >>> - sharp,ld-d5116z01b >> >> Ok to drop the sharp one I added. It should be able to be handled by >> the (newish) edp-panel, but I think the TI bridge driver needs some work >> for the specific platform (no I2C connection) to verify. > > Is the platform supported upstream? If so, which platform is it? Is > the TI bridge chip the ti-sn65dsi86? If so, I'm confused how you could > use that bridge chip without an i2c connection, but perhaps I'm > misunderstanding. :-P Yes, the platform is upstream. The 8998 laptops (clamshell). It is the ti-sn65si86. I suspect the I2C connection was not populated for cost reasons, then determined its much more convenient to have it as every generation after that I've seen has the I2C. If you check the datasheet closely, the I2C connection is optional. You can also configure the bridge inband using DSI commands. This is what the FW and Windows does. So, the DT binding needs to make the I2C property optional (this should be backwards compatible). The driver needs to detect that the I2C connection is not provided, and fall back to DSI commands. Regmap would be nice for this, but I got pushback on the proposal. Then I got sidetracked looking at other issues. -Jeff
Hi, On Fri, May 31, 2024 at 9:51 AM Jeffrey Hugo <quic_jhugo@quicinc.com> wrote: > > On 5/31/2024 10:20 AM, Doug Anderson wrote: > > Hi, > > > > On Fri, May 31, 2024 at 9:18 AM Jeffrey Hugo <quic_jhugo@quicinc.com> wrote: > >> > >> On 5/30/2024 5:12 PM, Dmitry Baryshkov wrote: > >>> There are two ways to describe an eDP panel in device tree. The > >>> recommended way is to add a device on the AUX bus, ideally using the > >>> edp-panel compatible. The legacy way is to define a top-level platform > >>> device for the panel. > >>> > >>> Document that adding support for eDP panels in a legacy way is strongly > >>> discouraged (if not forbidden at all). > >>> > >>> While we are at it, also drop legacy compatible strings and bindings for > >>> five panels. These compatible strings were never used by a DT file > >>> present in Linux kernel and most likely were never used with the > >>> upstream Linux kernel. > >>> > >>> The following compatibles were never used by the devices supported by > >>> the upstream kernel and are a subject to possible removal: > >>> > >>> - lg,lp097qx1-spa1 > >>> - samsung,lsn122dl01-c01 > >>> - sharp,ld-d5116z01b > >> > >> Ok to drop the sharp one I added. It should be able to be handled by > >> the (newish) edp-panel, but I think the TI bridge driver needs some work > >> for the specific platform (no I2C connection) to verify. > > > > Is the platform supported upstream? If so, which platform is it? Is > > the TI bridge chip the ti-sn65dsi86? If so, I'm confused how you could > > use that bridge chip without an i2c connection, but perhaps I'm > > misunderstanding. :-P > > Yes, the platform is upstream. The 8998 laptops (clamshell). It is the > ti-sn65si86. I suspect the I2C connection was not populated for cost > reasons, then determined its much more convenient to have it as every > generation after that I've seen has the I2C. > > If you check the datasheet closely, the I2C connection is optional. You > can also configure the bridge inband using DSI commands. This is what > the FW and Windows does. > > So, the DT binding needs to make the I2C property optional (this should > be backwards compatible). The driver needs to detect that the I2C > connection is not provided, and fall back to DSI commands. Regmap would > be nice for this, but I got pushback on the proposal. Then I got > sidetracked looking at other issues. Crazy! I'm sure I've skimmed over that part of the ti-sn65dsi86 datasheet before but I don't think I internalized it. I guess if you did it this way then you'd instantiate it as a platform device instead of an i2c device and that would be how you'd detect the difference. I could imagine this being a bit of a challenge to get working in the driver.
On Fri, May 31, 2024 at 10:18:07AM -0600, Jeffrey Hugo wrote: > On 5/30/2024 5:12 PM, Dmitry Baryshkov wrote: > > There are two ways to describe an eDP panel in device tree. The > > recommended way is to add a device on the AUX bus, ideally using the > > edp-panel compatible. The legacy way is to define a top-level platform > > device for the panel. > > > > Document that adding support for eDP panels in a legacy way is strongly > > discouraged (if not forbidden at all). > > > > While we are at it, also drop legacy compatible strings and bindings for > > five panels. These compatible strings were never used by a DT file > > present in Linux kernel and most likely were never used with the > > upstream Linux kernel. > > > > The following compatibles were never used by the devices supported by > > the upstream kernel and are a subject to possible removal: > > > > - lg,lp097qx1-spa1 > > - samsung,lsn122dl01-c01 > > - sharp,ld-d5116z01b > > Ok to drop the sharp one I added. It should be able to be handled by the > (newish) edp-panel, but I think the TI bridge driver needs some work for the > specific platform (no I2C connection) to verify. Thanks. I'm tempted to merge the series as is now and drop sharp,ld-d5116z01b once you can confirm that it can be handled by edp-panel on your platform.
On 5/31/2024 11:43 AM, Dmitry Baryshkov wrote: > On Fri, May 31, 2024 at 10:18:07AM -0600, Jeffrey Hugo wrote: >> On 5/30/2024 5:12 PM, Dmitry Baryshkov wrote: >>> There are two ways to describe an eDP panel in device tree. The >>> recommended way is to add a device on the AUX bus, ideally using the >>> edp-panel compatible. The legacy way is to define a top-level platform >>> device for the panel. >>> >>> Document that adding support for eDP panels in a legacy way is strongly >>> discouraged (if not forbidden at all). >>> >>> While we are at it, also drop legacy compatible strings and bindings for >>> five panels. These compatible strings were never used by a DT file >>> present in Linux kernel and most likely were never used with the >>> upstream Linux kernel. >>> >>> The following compatibles were never used by the devices supported by >>> the upstream kernel and are a subject to possible removal: >>> >>> - lg,lp097qx1-spa1 >>> - samsung,lsn122dl01-c01 >>> - sharp,ld-d5116z01b >> >> Ok to drop the sharp one I added. It should be able to be handled by the >> (newish) edp-panel, but I think the TI bridge driver needs some work for the >> specific platform (no I2C connection) to verify. > > Thanks. I'm tempted to merge the series as is now and drop > sharp,ld-d5116z01b once you can confirm that it can be handled by > edp-panel on your platform. > Sounds good to me. -Jeff
There are two ways to describe an eDP panel in device tree. The recommended way is to add a device on the AUX bus, ideally using the edp-panel compatible. The legacy way is to define a top-level platform device for the panel. Document that adding support for eDP panels in a legacy way is strongly discouraged (if not forbidden at all). While we are at it, also drop legacy compatible strings and bindings for five panels. These compatible strings were never used by a DT file present in Linux kernel and most likely were never used with the upstream Linux kernel. The following compatibles were never used by the devices supported by the upstream kernel and are a subject to possible removal: - lg,lp097qx1-spa1 - samsung,lsn122dl01-c01 - sharp,ld-d5116z01b I'm considering dropping them later, unless there is a good reason not to do so. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- Changes in v3: - Rephrased the warning comment, following some of Doug's suggestions. - Link to v2: https://lore.kernel.org/r/20240529-edp-panel-drop-v2-0-fcfc457fc8dd@linaro.org Changes in v2: - Actually drop support for five panels acked by Doug Andersson - Link to v1: https://lore.kernel.org/r/20240523-edp-panel-drop-v1-1-045d62511d09@linaro.org --- Dmitry Baryshkov (3): drm/panel-edp: add fat warning against adding new panel compatibles dt-bindings: display: panel-simple: drop several eDP panels drm/panel-edp: drop several legacy panels .../bindings/display/panel/panel-simple.yaml | 10 -- drivers/gpu/drm/panel/panel-edp.c | 192 +++------------------ 2 files changed, 25 insertions(+), 177 deletions(-) --- base-commit: 6dc544b66971c7f9909ff038b62149105272d26a change-id: 20240523-edp-panel-drop-00aa239b3c6b Best regards,