* [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-16 21:08 ` Douglas Anderson 0 siblings, 0 replies; 40+ messages in thread From: Douglas Anderson @ 2021-03-16 21:08 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring, Thierry Reding, Sam Ravnborg Cc: robdclark, Daniel Vetter, dri-devel, David Airlie, devicetree, swboyd, linux-arm-msm, Steev Klimaszewski, eballetbo, Andy Gross, Linus Walleij, drinkcat, Douglas Anderson, linux-kernel The sc7180-trogdor-pompom board might be attached to any number of a pile of eDP panels. At the moment I'm told that the list might include: - KD KD116N21-30NV-A010 - KD KD116N09-30NH-A016 - Starry 2081116HHD028001-51D - Sharp LQ116M1JW10 It should be noted that while the EDID programmed in the first 3 panels indicates that they should run with exactly the same timing (to keep things simple), the 4th panel not only needs different timing but has a different resolution. As is true in general with eDP panels, we can figure out which panel we have and all the info needed to drive its pixel clock by reading the EDID. However, we can do this only after we've powered the panel on. Powering on the panels requires following the timing diagram in each panel's datasheet which specifies delays between certain actions. This means that, while we can be quite dynamic about handling things we can't just totally skip out on describing the panel like we could do if it was connected to an external-facing DP port. While the different panels have slightly different delays, it's possible to come up with a set of unified delays that will work on all the panels. From reading the datasheets: * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 - HPD absent delay: 200 ms - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) * Starry 2081116HHD028001-51D - HPD absent delay: 100 ms - Enable delay: (link training done till enable BL): 200 ms - Unprepare delay: 500 ms * Sharp LQ116M1JW10 - HPD absent delay: 200 ms - Unprepare delay: 500 ms - Prepare to enable delay (power on till backlight): 100 ms Unified: - HPD absent delay: 200 ms - Unprepare delay: 500 ms - Enable delay: 200 ms NOTE: in theory the only thing that we _really_ need unity on is the "HPD absent delay" since once the panel asserts HPD we can read the EDID and could make per-panel decisions if we wanted. Let's create a definition of "a panel that can be attached to pompom" as a panel that provides a valid EDID and can work with the standard pompom power sequencing. If more panels are later attached to pompom then it's fine as long as they work in a compatible way. One might ask why we can't just use a generic string here and provide the timings directly in the device tree file. As I understand it, trying to describe generic power sequencing in the device tree is frowned upon and the one instance (SD/MMC) is regarded as a mistake that shouldn't be repeated. Specifying a power sequence per board (or per board class) feels like a reasonable compromise. We're not trying to define fully generic power sequence bindings but we can also take advantage of the semi-probable properties of the attached device. NOTE: I believe that past instances of supporting this type of thing have used the "white lie" approach. One representative panel was listed in the device tree. The power sequencings of this representative panel were OK to use across all panels that might be attached and other differences were handled by EDID. This patch attempts to set a new precedent and avoid the need for the white lie. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- .../devicetree/bindings/display/panel/panel-simple.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 62b0d54d87b7..9807dbc1cceb 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -140,6 +140,10 @@ properties: - giantplus,gpg48273qs5 # GiantPlus GPM940B0 3.0" QVGA TFT LCD panel - giantplus,gpm940b0 + # A panel connected to a google,pompom board. Panel is guaranteed to + # confirm to google,pompom-panel power sequencing requirements and then + # the specific panel will be probed via EDID. + - google,pompom-panel # HannStar Display Corp. HSD070PWW1 7.0" WXGA TFT LCD panel - hannstar,hsd070pww1 # HannStar Display Corp. HSD100PXN1 10.1" XGA LVDS panel -- 2.31.0.rc2.261.g7f71774620-goog ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-16 21:08 ` Douglas Anderson 0 siblings, 0 replies; 40+ messages in thread From: Douglas Anderson @ 2021-03-16 21:08 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring, Thierry Reding, Sam Ravnborg Cc: robdclark, devicetree, drinkcat, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, swboyd, Andy Gross, Steev Klimaszewski, linux-kernel The sc7180-trogdor-pompom board might be attached to any number of a pile of eDP panels. At the moment I'm told that the list might include: - KD KD116N21-30NV-A010 - KD KD116N09-30NH-A016 - Starry 2081116HHD028001-51D - Sharp LQ116M1JW10 It should be noted that while the EDID programmed in the first 3 panels indicates that they should run with exactly the same timing (to keep things simple), the 4th panel not only needs different timing but has a different resolution. As is true in general with eDP panels, we can figure out which panel we have and all the info needed to drive its pixel clock by reading the EDID. However, we can do this only after we've powered the panel on. Powering on the panels requires following the timing diagram in each panel's datasheet which specifies delays between certain actions. This means that, while we can be quite dynamic about handling things we can't just totally skip out on describing the panel like we could do if it was connected to an external-facing DP port. While the different panels have slightly different delays, it's possible to come up with a set of unified delays that will work on all the panels. From reading the datasheets: * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 - HPD absent delay: 200 ms - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) * Starry 2081116HHD028001-51D - HPD absent delay: 100 ms - Enable delay: (link training done till enable BL): 200 ms - Unprepare delay: 500 ms * Sharp LQ116M1JW10 - HPD absent delay: 200 ms - Unprepare delay: 500 ms - Prepare to enable delay (power on till backlight): 100 ms Unified: - HPD absent delay: 200 ms - Unprepare delay: 500 ms - Enable delay: 200 ms NOTE: in theory the only thing that we _really_ need unity on is the "HPD absent delay" since once the panel asserts HPD we can read the EDID and could make per-panel decisions if we wanted. Let's create a definition of "a panel that can be attached to pompom" as a panel that provides a valid EDID and can work with the standard pompom power sequencing. If more panels are later attached to pompom then it's fine as long as they work in a compatible way. One might ask why we can't just use a generic string here and provide the timings directly in the device tree file. As I understand it, trying to describe generic power sequencing in the device tree is frowned upon and the one instance (SD/MMC) is regarded as a mistake that shouldn't be repeated. Specifying a power sequence per board (or per board class) feels like a reasonable compromise. We're not trying to define fully generic power sequence bindings but we can also take advantage of the semi-probable properties of the attached device. NOTE: I believe that past instances of supporting this type of thing have used the "white lie" approach. One representative panel was listed in the device tree. The power sequencings of this representative panel were OK to use across all panels that might be attached and other differences were handled by EDID. This patch attempts to set a new precedent and avoid the need for the white lie. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- .../devicetree/bindings/display/panel/panel-simple.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 62b0d54d87b7..9807dbc1cceb 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -140,6 +140,10 @@ properties: - giantplus,gpg48273qs5 # GiantPlus GPM940B0 3.0" QVGA TFT LCD panel - giantplus,gpm940b0 + # A panel connected to a google,pompom board. Panel is guaranteed to + # confirm to google,pompom-panel power sequencing requirements and then + # the specific panel will be probed via EDID. + - google,pompom-panel # HannStar Display Corp. HSD070PWW1 7.0" WXGA TFT LCD panel - hannstar,hsd070pww1 # HannStar Display Corp. HSD100PXN1 10.1" XGA LVDS panel -- 2.31.0.rc2.261.g7f71774620-goog _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 2/3] drm/panel: panel-simple: Add the panel on sc7180-trogdor-pompom 2021-03-16 21:08 ` Douglas Anderson @ 2021-03-16 21:08 ` Douglas Anderson -1 siblings, 0 replies; 40+ messages in thread From: Douglas Anderson @ 2021-03-16 21:08 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring, Thierry Reding, Sam Ravnborg Cc: robdclark, Daniel Vetter, dri-devel, David Airlie, devicetree, swboyd, linux-arm-msm, Steev Klimaszewski, eballetbo, Andy Gross, Linus Walleij, drinkcat, Douglas Anderson, linux-kernel This adds an entry to simple panel for the timings of a panel that might be attached to the board sc7180-trogdor-pompom. As talked about in the previous patch ("dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom") any number of panels might be attached. These panels might have different timings or even different resolutions. When we see a pompom panel we'll use some conservative power sequence timings that can work with any of the panels that will be attached. All the rest of the details about the panel can be found by read its EDID. NOTE: if we wanted to optimize things a bit we could figure out which of the panels was actually attached and perhaps reduce some of the timings by a tiny bit if it was a panel that we recognized. I won't attempt this right now but it could be a nice future optimization, epsecially getting rid of the 200 ms "enable delay" imposed by the Starry panel. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- For overall discussion about the idea of defining a generic panel at the board level, please respond in the bindings patch so we can keep all discussion in one place. drivers/gpu/drm/panel/panel-simple.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 4e2dad314c79..c8964efd562b 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -2205,6 +2205,15 @@ static const struct panel_desc hitachi_tx23d38vm0caa = { }, }; +static const struct panel_desc google_pompom_panel = { + .delay = { + .hpd_absent_delay = 200, + .enable = 200, + .unprepare = 500, + }, + .connector_type = DRM_MODE_CONNECTOR_eDP, +}; + static const struct drm_display_mode innolux_at043tn24_mode = { .clock = 9000, .hdisplay = 480, @@ -4265,6 +4274,9 @@ static const struct of_device_id platform_of_match[] = { }, { .compatible = "hit,tx23d38vm0caa", .data = &hitachi_tx23d38vm0caa + }, { + .compatible = "google,pompom-panel", + .data = &google_pompom_panel, }, { .compatible = "innolux,at043tn24", .data = &innolux_at043tn24, -- 2.31.0.rc2.261.g7f71774620-goog ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 2/3] drm/panel: panel-simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-16 21:08 ` Douglas Anderson 0 siblings, 0 replies; 40+ messages in thread From: Douglas Anderson @ 2021-03-16 21:08 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring, Thierry Reding, Sam Ravnborg Cc: robdclark, devicetree, drinkcat, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, swboyd, Andy Gross, Steev Klimaszewski, linux-kernel This adds an entry to simple panel for the timings of a panel that might be attached to the board sc7180-trogdor-pompom. As talked about in the previous patch ("dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom") any number of panels might be attached. These panels might have different timings or even different resolutions. When we see a pompom panel we'll use some conservative power sequence timings that can work with any of the panels that will be attached. All the rest of the details about the panel can be found by read its EDID. NOTE: if we wanted to optimize things a bit we could figure out which of the panels was actually attached and perhaps reduce some of the timings by a tiny bit if it was a panel that we recognized. I won't attempt this right now but it could be a nice future optimization, epsecially getting rid of the 200 ms "enable delay" imposed by the Starry panel. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- For overall discussion about the idea of defining a generic panel at the board level, please respond in the bindings patch so we can keep all discussion in one place. drivers/gpu/drm/panel/panel-simple.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 4e2dad314c79..c8964efd562b 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -2205,6 +2205,15 @@ static const struct panel_desc hitachi_tx23d38vm0caa = { }, }; +static const struct panel_desc google_pompom_panel = { + .delay = { + .hpd_absent_delay = 200, + .enable = 200, + .unprepare = 500, + }, + .connector_type = DRM_MODE_CONNECTOR_eDP, +}; + static const struct drm_display_mode innolux_at043tn24_mode = { .clock = 9000, .hdisplay = 480, @@ -4265,6 +4274,9 @@ static const struct of_device_id platform_of_match[] = { }, { .compatible = "hit,tx23d38vm0caa", .data = &hitachi_tx23d38vm0caa + }, { + .compatible = "google,pompom-panel", + .data = &google_pompom_panel, }, { .compatible = "innolux,at043tn24", .data = &innolux_at043tn24, -- 2.31.0.rc2.261.g7f71774620-goog _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 3/3] arm64: dts: qcom: Don't specify exactly what panel is on pompom 2021-03-16 21:08 ` Douglas Anderson @ 2021-03-16 21:08 ` Douglas Anderson -1 siblings, 0 replies; 40+ messages in thread From: Douglas Anderson @ 2021-03-16 21:08 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring, Thierry Reding, Sam Ravnborg Cc: robdclark, Daniel Vetter, dri-devel, David Airlie, devicetree, swboyd, linux-arm-msm, Steev Klimaszewski, eballetbo, Andy Gross, Linus Walleij, drinkcat, Douglas Anderson, linux-kernel As talked about in the previous patch ("dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom") any number of panels might be attached. Let's stop specifying a single panel and just specify that we have a "pompom panel". Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Before landing this we need the patch ("drm/panel: panel-simple: Add the panel on sc7180-trogdor-pompom") from earlier in this series. Less obviously, though, we also need the patch ("drm/bridge: ti-sn65dsi86: Properly get the EDID, but only if refclk") [1]. [1] https://lore.kernel.org/r/20210304155144.3.I60a7fb23ce4589006bc95c64ab8d15c74b876e68@changeid arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi index d253a08f6fc8..f98c18f6ff94 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi @@ -108,7 +108,7 @@ ap_ts: touchscreen@10 { }; &panel { - compatible = "kingdisplay,kd116n21-30nv-a010"; + compatible = "google,pompom-panel"; }; &pen_insert { -- 2.31.0.rc2.261.g7f71774620-goog ^ permalink raw reply related [flat|nested] 40+ messages in thread
* [RFC PATCH 3/3] arm64: dts: qcom: Don't specify exactly what panel is on pompom @ 2021-03-16 21:08 ` Douglas Anderson 0 siblings, 0 replies; 40+ messages in thread From: Douglas Anderson @ 2021-03-16 21:08 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring, Thierry Reding, Sam Ravnborg Cc: robdclark, devicetree, drinkcat, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, swboyd, Andy Gross, Steev Klimaszewski, linux-kernel As talked about in the previous patch ("dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom") any number of panels might be attached. Let's stop specifying a single panel and just specify that we have a "pompom panel". Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Before landing this we need the patch ("drm/panel: panel-simple: Add the panel on sc7180-trogdor-pompom") from earlier in this series. Less obviously, though, we also need the patch ("drm/bridge: ti-sn65dsi86: Properly get the EDID, but only if refclk") [1]. [1] https://lore.kernel.org/r/20210304155144.3.I60a7fb23ce4589006bc95c64ab8d15c74b876e68@changeid arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi index d253a08f6fc8..f98c18f6ff94 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi @@ -108,7 +108,7 @@ ap_ts: touchscreen@10 { }; &panel { - compatible = "kingdisplay,kd116n21-30nv-a010"; + compatible = "google,pompom-panel"; }; &pen_insert { -- 2.31.0.rc2.261.g7f71774620-goog _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-16 21:08 ` Douglas Anderson @ 2021-03-17 23:27 ` Matthias Kaehlcke -1 siblings, 0 replies; 40+ messages in thread From: Matthias Kaehlcke @ 2021-03-17 23:27 UTC (permalink / raw) To: Douglas Anderson Cc: Bjorn Andersson, Rob Herring, Thierry Reding, Sam Ravnborg, robdclark, Daniel Vetter, dri-devel, David Airlie, devicetree, swboyd, linux-arm-msm, Steev Klimaszewski, eballetbo, Andy Gross, Linus Walleij, drinkcat, linux-kernel On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > The sc7180-trogdor-pompom board might be attached to any number of a > pile of eDP panels. At the moment I'm told that the list might include: > - KD KD116N21-30NV-A010 > - KD KD116N09-30NH-A016 > - Starry 2081116HHD028001-51D > - Sharp LQ116M1JW10 > > It should be noted that while the EDID programmed in the first 3 > panels indicates that they should run with exactly the same timing (to > keep things simple), the 4th panel not only needs different timing but > has a different resolution. > > As is true in general with eDP panels, we can figure out which panel > we have and all the info needed to drive its pixel clock by reading > the EDID. However, we can do this only after we've powered the panel > on. Powering on the panels requires following the timing diagram in > each panel's datasheet which specifies delays between certain > actions. This means that, while we can be quite dynamic about handling > things we can't just totally skip out on describing the panel like we > could do if it was connected to an external-facing DP port. > > While the different panels have slightly different delays, it's > possible to come up with a set of unified delays that will work on all > the panels. From reading the datasheets: > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > - HPD absent delay: 200 ms > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > * Starry 2081116HHD028001-51D > - HPD absent delay: 100 ms > - Enable delay: (link training done till enable BL): 200 ms > - Unprepare delay: 500 ms > * Sharp LQ116M1JW10 > - HPD absent delay: 200 ms > - Unprepare delay: 500 ms > - Prepare to enable delay (power on till backlight): 100 ms > > Unified: > - HPD absent delay: 200 ms > - Unprepare delay: 500 ms > - Enable delay: 200 ms > > NOTE: in theory the only thing that we _really_ need unity on is the > "HPD absent delay" since once the panel asserts HPD we can read the > EDID and could make per-panel decisions if we wanted. > > Let's create a definition of "a panel that can be attached to pompom" > as a panel that provides a valid EDID and can work with the standard > pompom power sequencing. If more panels are later attached to pompom > then it's fine as long as they work in a compatible way. > > One might ask why we can't just use a generic string here and provide > the timings directly in the device tree file. As I understand it, > trying to describe generic power sequencing in the device tree is > frowned upon and the one instance (SD/MMC) is regarded as a mistake > that shouldn't be repeated. Specifying a power sequence per board (or > per board class) feels like a reasonable compromise. We're not trying > to define fully generic power sequence bindings but we can also take > advantage of the semi-probable properties of the attached device. > > NOTE: I believe that past instances of supporting this type of thing > have used the "white lie" approach. One representative panel was > listed in the device tree. The power sequencings of this > representative panel were OK to use across all panels that might be > attached and other differences were handled by EDID. This patch > attempts to set a new precedent and avoid the need for the white lie. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- Sounds reasonable to me if DT maintainers can live with this abstract hardware definition. It's clearer than the 'white lie' approach. It's then up to the vendor/manufacturer to ensure to only ship devices with panels that have compatible timings. > .../devicetree/bindings/display/panel/panel-simple.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > index 62b0d54d87b7..9807dbc1cceb 100644 > --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > @@ -140,6 +140,10 @@ properties: > - giantplus,gpg48273qs5 > # GiantPlus GPM940B0 3.0" QVGA TFT LCD panel > - giantplus,gpm940b0 > + # A panel connected to a google,pompom board. Panel is guaranteed to > + # confirm to google,pompom-panel power sequencing requirements and then s/confirm/conform/ ? > + # the specific panel will be probed via EDID. > + - google,pompom-panel > # HannStar Display Corp. HSD070PWW1 7.0" WXGA TFT LCD panel > - hannstar,hsd070pww1 > # HannStar Display Corp. HSD100PXN1 10.1" XGA LVDS panel FWIW: Reviewed-by: Matthias Kaehlcke <mka@chromium.org> ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-17 23:27 ` Matthias Kaehlcke 0 siblings, 0 replies; 40+ messages in thread From: Matthias Kaehlcke @ 2021-03-17 23:27 UTC (permalink / raw) To: Douglas Anderson Cc: robdclark, devicetree, drinkcat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Rob Herring, Thierry Reding, Steev Klimaszewski, swboyd, Sam Ravnborg, linux-kernel On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > The sc7180-trogdor-pompom board might be attached to any number of a > pile of eDP panels. At the moment I'm told that the list might include: > - KD KD116N21-30NV-A010 > - KD KD116N09-30NH-A016 > - Starry 2081116HHD028001-51D > - Sharp LQ116M1JW10 > > It should be noted that while the EDID programmed in the first 3 > panels indicates that they should run with exactly the same timing (to > keep things simple), the 4th panel not only needs different timing but > has a different resolution. > > As is true in general with eDP panels, we can figure out which panel > we have and all the info needed to drive its pixel clock by reading > the EDID. However, we can do this only after we've powered the panel > on. Powering on the panels requires following the timing diagram in > each panel's datasheet which specifies delays between certain > actions. This means that, while we can be quite dynamic about handling > things we can't just totally skip out on describing the panel like we > could do if it was connected to an external-facing DP port. > > While the different panels have slightly different delays, it's > possible to come up with a set of unified delays that will work on all > the panels. From reading the datasheets: > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > - HPD absent delay: 200 ms > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > * Starry 2081116HHD028001-51D > - HPD absent delay: 100 ms > - Enable delay: (link training done till enable BL): 200 ms > - Unprepare delay: 500 ms > * Sharp LQ116M1JW10 > - HPD absent delay: 200 ms > - Unprepare delay: 500 ms > - Prepare to enable delay (power on till backlight): 100 ms > > Unified: > - HPD absent delay: 200 ms > - Unprepare delay: 500 ms > - Enable delay: 200 ms > > NOTE: in theory the only thing that we _really_ need unity on is the > "HPD absent delay" since once the panel asserts HPD we can read the > EDID and could make per-panel decisions if we wanted. > > Let's create a definition of "a panel that can be attached to pompom" > as a panel that provides a valid EDID and can work with the standard > pompom power sequencing. If more panels are later attached to pompom > then it's fine as long as they work in a compatible way. > > One might ask why we can't just use a generic string here and provide > the timings directly in the device tree file. As I understand it, > trying to describe generic power sequencing in the device tree is > frowned upon and the one instance (SD/MMC) is regarded as a mistake > that shouldn't be repeated. Specifying a power sequence per board (or > per board class) feels like a reasonable compromise. We're not trying > to define fully generic power sequence bindings but we can also take > advantage of the semi-probable properties of the attached device. > > NOTE: I believe that past instances of supporting this type of thing > have used the "white lie" approach. One representative panel was > listed in the device tree. The power sequencings of this > representative panel were OK to use across all panels that might be > attached and other differences were handled by EDID. This patch > attempts to set a new precedent and avoid the need for the white lie. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- Sounds reasonable to me if DT maintainers can live with this abstract hardware definition. It's clearer than the 'white lie' approach. It's then up to the vendor/manufacturer to ensure to only ship devices with panels that have compatible timings. > .../devicetree/bindings/display/panel/panel-simple.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > index 62b0d54d87b7..9807dbc1cceb 100644 > --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > @@ -140,6 +140,10 @@ properties: > - giantplus,gpg48273qs5 > # GiantPlus GPM940B0 3.0" QVGA TFT LCD panel > - giantplus,gpm940b0 > + # A panel connected to a google,pompom board. Panel is guaranteed to > + # confirm to google,pompom-panel power sequencing requirements and then s/confirm/conform/ ? > + # the specific panel will be probed via EDID. > + - google,pompom-panel > # HannStar Display Corp. HSD070PWW1 7.0" WXGA TFT LCD panel > - hannstar,hsd070pww1 > # HannStar Display Corp. HSD100PXN1 10.1" XGA LVDS panel FWIW: Reviewed-by: Matthias Kaehlcke <mka@chromium.org> _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-17 23:27 ` Matthias Kaehlcke @ 2021-03-18 1:53 ` Rob Clark -1 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-18 1:53 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Rob Herring, Thierry Reding, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > The sc7180-trogdor-pompom board might be attached to any number of a > > pile of eDP panels. At the moment I'm told that the list might include: > > - KD KD116N21-30NV-A010 > > - KD KD116N09-30NH-A016 > > - Starry 2081116HHD028001-51D > > - Sharp LQ116M1JW10 > > > > It should be noted that while the EDID programmed in the first 3 > > panels indicates that they should run with exactly the same timing (to > > keep things simple), the 4th panel not only needs different timing but > > has a different resolution. > > > > As is true in general with eDP panels, we can figure out which panel > > we have and all the info needed to drive its pixel clock by reading > > the EDID. However, we can do this only after we've powered the panel > > on. Powering on the panels requires following the timing diagram in > > each panel's datasheet which specifies delays between certain > > actions. This means that, while we can be quite dynamic about handling > > things we can't just totally skip out on describing the panel like we > > could do if it was connected to an external-facing DP port. > > > > While the different panels have slightly different delays, it's > > possible to come up with a set of unified delays that will work on all > > the panels. From reading the datasheets: > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > - HPD absent delay: 200 ms > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > * Starry 2081116HHD028001-51D > > - HPD absent delay: 100 ms > > - Enable delay: (link training done till enable BL): 200 ms > > - Unprepare delay: 500 ms > > * Sharp LQ116M1JW10 > > - HPD absent delay: 200 ms > > - Unprepare delay: 500 ms > > - Prepare to enable delay (power on till backlight): 100 ms > > > > Unified: > > - HPD absent delay: 200 ms > > - Unprepare delay: 500 ms > > - Enable delay: 200 ms > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > "HPD absent delay" since once the panel asserts HPD we can read the > > EDID and could make per-panel decisions if we wanted. > > > > Let's create a definition of "a panel that can be attached to pompom" > > as a panel that provides a valid EDID and can work with the standard > > pompom power sequencing. If more panels are later attached to pompom > > then it's fine as long as they work in a compatible way. > > > > One might ask why we can't just use a generic string here and provide > > the timings directly in the device tree file. As I understand it, > > trying to describe generic power sequencing in the device tree is > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > that shouldn't be repeated. Specifying a power sequence per board (or > > per board class) feels like a reasonable compromise. We're not trying > > to define fully generic power sequence bindings but we can also take > > advantage of the semi-probable properties of the attached device. > > > > NOTE: I believe that past instances of supporting this type of thing > > have used the "white lie" approach. One representative panel was > > listed in the device tree. The power sequencings of this > > representative panel were OK to use across all panels that might be > > attached and other differences were handled by EDID. This patch > > attempts to set a new precedent and avoid the need for the white lie. > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > Sounds reasonable to me if DT maintainers can live with this abstract > hardware definition. It's clearer than the 'white lie' approach. Yeah, it is a weird grey area between "discoverable" and "not discoverable".. but I favor DT reflecting reality as much as possible/feasible, so I think this is definity cleaner than "white lies" Reviewed-by: Rob Clark <robdclark@gmail.com> > It's then up to the vendor/manufacturer to ensure to only ship devices > with panels that have compatible timings. > > > .../devicetree/bindings/display/panel/panel-simple.yaml | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > > index 62b0d54d87b7..9807dbc1cceb 100644 > > --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > > @@ -140,6 +140,10 @@ properties: > > - giantplus,gpg48273qs5 > > # GiantPlus GPM940B0 3.0" QVGA TFT LCD panel > > - giantplus,gpm940b0 > > + # A panel connected to a google,pompom board. Panel is guaranteed to > > + # confirm to google,pompom-panel power sequencing requirements and then > > s/confirm/conform/ ? > > > + # the specific panel will be probed via EDID. > > + - google,pompom-panel > > # HannStar Display Corp. HSD070PWW1 7.0" WXGA TFT LCD panel > > - hannstar,hsd070pww1 > > # HannStar Display Corp. HSD100PXN1 10.1" XGA LVDS panel > > FWIW: > > Reviewed-by: Matthias Kaehlcke <mka@chromium.org> > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-18 1:53 ` Rob Clark 0 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-18 1:53 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Bjorn Andersson, Rob Herring, Andy Gross, Thierry Reding, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > The sc7180-trogdor-pompom board might be attached to any number of a > > pile of eDP panels. At the moment I'm told that the list might include: > > - KD KD116N21-30NV-A010 > > - KD KD116N09-30NH-A016 > > - Starry 2081116HHD028001-51D > > - Sharp LQ116M1JW10 > > > > It should be noted that while the EDID programmed in the first 3 > > panels indicates that they should run with exactly the same timing (to > > keep things simple), the 4th panel not only needs different timing but > > has a different resolution. > > > > As is true in general with eDP panels, we can figure out which panel > > we have and all the info needed to drive its pixel clock by reading > > the EDID. However, we can do this only after we've powered the panel > > on. Powering on the panels requires following the timing diagram in > > each panel's datasheet which specifies delays between certain > > actions. This means that, while we can be quite dynamic about handling > > things we can't just totally skip out on describing the panel like we > > could do if it was connected to an external-facing DP port. > > > > While the different panels have slightly different delays, it's > > possible to come up with a set of unified delays that will work on all > > the panels. From reading the datasheets: > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > - HPD absent delay: 200 ms > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > * Starry 2081116HHD028001-51D > > - HPD absent delay: 100 ms > > - Enable delay: (link training done till enable BL): 200 ms > > - Unprepare delay: 500 ms > > * Sharp LQ116M1JW10 > > - HPD absent delay: 200 ms > > - Unprepare delay: 500 ms > > - Prepare to enable delay (power on till backlight): 100 ms > > > > Unified: > > - HPD absent delay: 200 ms > > - Unprepare delay: 500 ms > > - Enable delay: 200 ms > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > "HPD absent delay" since once the panel asserts HPD we can read the > > EDID and could make per-panel decisions if we wanted. > > > > Let's create a definition of "a panel that can be attached to pompom" > > as a panel that provides a valid EDID and can work with the standard > > pompom power sequencing. If more panels are later attached to pompom > > then it's fine as long as they work in a compatible way. > > > > One might ask why we can't just use a generic string here and provide > > the timings directly in the device tree file. As I understand it, > > trying to describe generic power sequencing in the device tree is > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > that shouldn't be repeated. Specifying a power sequence per board (or > > per board class) feels like a reasonable compromise. We're not trying > > to define fully generic power sequence bindings but we can also take > > advantage of the semi-probable properties of the attached device. > > > > NOTE: I believe that past instances of supporting this type of thing > > have used the "white lie" approach. One representative panel was > > listed in the device tree. The power sequencings of this > > representative panel were OK to use across all panels that might be > > attached and other differences were handled by EDID. This patch > > attempts to set a new precedent and avoid the need for the white lie. > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > Sounds reasonable to me if DT maintainers can live with this abstract > hardware definition. It's clearer than the 'white lie' approach. Yeah, it is a weird grey area between "discoverable" and "not discoverable".. but I favor DT reflecting reality as much as possible/feasible, so I think this is definity cleaner than "white lies" Reviewed-by: Rob Clark <robdclark@gmail.com> > It's then up to the vendor/manufacturer to ensure to only ship devices > with panels that have compatible timings. > > > .../devicetree/bindings/display/panel/panel-simple.yaml | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > > index 62b0d54d87b7..9807dbc1cceb 100644 > > --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > > @@ -140,6 +140,10 @@ properties: > > - giantplus,gpg48273qs5 > > # GiantPlus GPM940B0 3.0" QVGA TFT LCD panel > > - giantplus,gpm940b0 > > + # A panel connected to a google,pompom board. Panel is guaranteed to > > + # confirm to google,pompom-panel power sequencing requirements and then > > s/confirm/conform/ ? > > > + # the specific panel will be probed via EDID. > > + - google,pompom-panel > > # HannStar Display Corp. HSD070PWW1 7.0" WXGA TFT LCD panel > > - hannstar,hsd070pww1 > > # HannStar Display Corp. HSD100PXN1 10.1" XGA LVDS panel > > FWIW: > > Reviewed-by: Matthias Kaehlcke <mka@chromium.org> > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-18 1:53 ` Rob Clark @ 2021-03-26 12:39 ` Thierry Reding -1 siblings, 0 replies; 40+ messages in thread From: Thierry Reding @ 2021-03-26 12:39 UTC (permalink / raw) To: Rob Clark Cc: Matthias Kaehlcke, Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Rob Herring, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List [-- Attachment #1: Type: text/plain, Size: 5612 bytes --] On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > pile of eDP panels. At the moment I'm told that the list might include: > > > - KD KD116N21-30NV-A010 > > > - KD KD116N09-30NH-A016 > > > - Starry 2081116HHD028001-51D > > > - Sharp LQ116M1JW10 > > > > > > It should be noted that while the EDID programmed in the first 3 > > > panels indicates that they should run with exactly the same timing (to > > > keep things simple), the 4th panel not only needs different timing but > > > has a different resolution. > > > > > > As is true in general with eDP panels, we can figure out which panel > > > we have and all the info needed to drive its pixel clock by reading > > > the EDID. However, we can do this only after we've powered the panel > > > on. Powering on the panels requires following the timing diagram in > > > each panel's datasheet which specifies delays between certain > > > actions. This means that, while we can be quite dynamic about handling > > > things we can't just totally skip out on describing the panel like we > > > could do if it was connected to an external-facing DP port. > > > > > > While the different panels have slightly different delays, it's > > > possible to come up with a set of unified delays that will work on all > > > the panels. From reading the datasheets: > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > - HPD absent delay: 200 ms > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > * Starry 2081116HHD028001-51D > > > - HPD absent delay: 100 ms > > > - Enable delay: (link training done till enable BL): 200 ms > > > - Unprepare delay: 500 ms > > > * Sharp LQ116M1JW10 > > > - HPD absent delay: 200 ms > > > - Unprepare delay: 500 ms > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > Unified: > > > - HPD absent delay: 200 ms > > > - Unprepare delay: 500 ms > > > - Enable delay: 200 ms > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > EDID and could make per-panel decisions if we wanted. > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > as a panel that provides a valid EDID and can work with the standard > > > pompom power sequencing. If more panels are later attached to pompom > > > then it's fine as long as they work in a compatible way. > > > > > > One might ask why we can't just use a generic string here and provide > > > the timings directly in the device tree file. As I understand it, > > > trying to describe generic power sequencing in the device tree is > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > per board class) feels like a reasonable compromise. We're not trying > > > to define fully generic power sequence bindings but we can also take > > > advantage of the semi-probable properties of the attached device. > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > have used the "white lie" approach. One representative panel was > > > listed in the device tree. The power sequencings of this > > > representative panel were OK to use across all panels that might be > > > attached and other differences were handled by EDID. This patch > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > --- > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > hardware definition. It's clearer than the 'white lie' approach. > > Yeah, it is a weird grey area between "discoverable" and "not > discoverable".. but I favor DT reflecting reality as much as > possible/feasible, so I think this is definity cleaner than "white > lies" This is practically no different than the "white lie". I suppose you could perhaps call it "more honest", if you want. The point remains that unless we describe exactly which panel we're dealing with, we ultimately have no way of properly quirking anything if we ever have to. Also, once we allow this kind of wildcard we can suddenly get into a situation where people might want to reuse this on something that's not at all a google-pompom board because the same particular power sequence happens to work on on some other board. Similarly I can imagine a situation where we could now have the same panel supported by multiple different wildcard compatible strings. How is that supposed to be any cleaner than what we have now? And I still keep wondering why bootloaders can't be taught about these kinds of things. We have in the past discussed various solutions where the bootloader could detect the type of panel connected and set the proper compatible string. If that's too complicated and these really are standardized interfaces that work across a wide range of devices with perhaps a couple of standard parameter, then introducing a standard connector type like Rob Herring is suggesting makes more sense because that more properly describes where exactly the standardization is going on (i.e. at the interface level rather than the panel level). Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-26 12:39 ` Thierry Reding 0 siblings, 0 replies; 40+ messages in thread From: Thierry Reding @ 2021-03-26 12:39 UTC (permalink / raw) To: Rob Clark Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, Linux Kernel Mailing List, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Bjorn Andersson, Rob Herring, Matthias Kaehlcke, Andy Gross, Stephen Boyd, Sam Ravnborg, Steev Klimaszewski [-- Attachment #1.1: Type: text/plain, Size: 5612 bytes --] On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > pile of eDP panels. At the moment I'm told that the list might include: > > > - KD KD116N21-30NV-A010 > > > - KD KD116N09-30NH-A016 > > > - Starry 2081116HHD028001-51D > > > - Sharp LQ116M1JW10 > > > > > > It should be noted that while the EDID programmed in the first 3 > > > panels indicates that they should run with exactly the same timing (to > > > keep things simple), the 4th panel not only needs different timing but > > > has a different resolution. > > > > > > As is true in general with eDP panels, we can figure out which panel > > > we have and all the info needed to drive its pixel clock by reading > > > the EDID. However, we can do this only after we've powered the panel > > > on. Powering on the panels requires following the timing diagram in > > > each panel's datasheet which specifies delays between certain > > > actions. This means that, while we can be quite dynamic about handling > > > things we can't just totally skip out on describing the panel like we > > > could do if it was connected to an external-facing DP port. > > > > > > While the different panels have slightly different delays, it's > > > possible to come up with a set of unified delays that will work on all > > > the panels. From reading the datasheets: > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > - HPD absent delay: 200 ms > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > * Starry 2081116HHD028001-51D > > > - HPD absent delay: 100 ms > > > - Enable delay: (link training done till enable BL): 200 ms > > > - Unprepare delay: 500 ms > > > * Sharp LQ116M1JW10 > > > - HPD absent delay: 200 ms > > > - Unprepare delay: 500 ms > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > Unified: > > > - HPD absent delay: 200 ms > > > - Unprepare delay: 500 ms > > > - Enable delay: 200 ms > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > EDID and could make per-panel decisions if we wanted. > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > as a panel that provides a valid EDID and can work with the standard > > > pompom power sequencing. If more panels are later attached to pompom > > > then it's fine as long as they work in a compatible way. > > > > > > One might ask why we can't just use a generic string here and provide > > > the timings directly in the device tree file. As I understand it, > > > trying to describe generic power sequencing in the device tree is > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > per board class) feels like a reasonable compromise. We're not trying > > > to define fully generic power sequence bindings but we can also take > > > advantage of the semi-probable properties of the attached device. > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > have used the "white lie" approach. One representative panel was > > > listed in the device tree. The power sequencings of this > > > representative panel were OK to use across all panels that might be > > > attached and other differences were handled by EDID. This patch > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > --- > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > hardware definition. It's clearer than the 'white lie' approach. > > Yeah, it is a weird grey area between "discoverable" and "not > discoverable".. but I favor DT reflecting reality as much as > possible/feasible, so I think this is definity cleaner than "white > lies" This is practically no different than the "white lie". I suppose you could perhaps call it "more honest", if you want. The point remains that unless we describe exactly which panel we're dealing with, we ultimately have no way of properly quirking anything if we ever have to. Also, once we allow this kind of wildcard we can suddenly get into a situation where people might want to reuse this on something that's not at all a google-pompom board because the same particular power sequence happens to work on on some other board. Similarly I can imagine a situation where we could now have the same panel supported by multiple different wildcard compatible strings. How is that supposed to be any cleaner than what we have now? And I still keep wondering why bootloaders can't be taught about these kinds of things. We have in the past discussed various solutions where the bootloader could detect the type of panel connected and set the proper compatible string. If that's too complicated and these really are standardized interfaces that work across a wide range of devices with perhaps a couple of standard parameter, then introducing a standard connector type like Rob Herring is suggesting makes more sense because that more properly describes where exactly the standardization is going on (i.e. at the interface level rather than the panel level). Thierry [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 12:39 ` Thierry Reding @ 2021-03-26 15:18 ` Rob Clark -1 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-26 15:18 UTC (permalink / raw) To: Thierry Reding Cc: Matthias Kaehlcke, Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Rob Herring, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > - KD KD116N21-30NV-A010 > > > > - KD KD116N09-30NH-A016 > > > > - Starry 2081116HHD028001-51D > > > > - Sharp LQ116M1JW10 > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > panels indicates that they should run with exactly the same timing (to > > > > keep things simple), the 4th panel not only needs different timing but > > > > has a different resolution. > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > we have and all the info needed to drive its pixel clock by reading > > > > the EDID. However, we can do this only after we've powered the panel > > > > on. Powering on the panels requires following the timing diagram in > > > > each panel's datasheet which specifies delays between certain > > > > actions. This means that, while we can be quite dynamic about handling > > > > things we can't just totally skip out on describing the panel like we > > > > could do if it was connected to an external-facing DP port. > > > > > > > > While the different panels have slightly different delays, it's > > > > possible to come up with a set of unified delays that will work on all > > > > the panels. From reading the datasheets: > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > - HPD absent delay: 200 ms > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > * Starry 2081116HHD028001-51D > > > > - HPD absent delay: 100 ms > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > - Unprepare delay: 500 ms > > > > * Sharp LQ116M1JW10 > > > > - HPD absent delay: 200 ms > > > > - Unprepare delay: 500 ms > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > Unified: > > > > - HPD absent delay: 200 ms > > > > - Unprepare delay: 500 ms > > > > - Enable delay: 200 ms > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > as a panel that provides a valid EDID and can work with the standard > > > > pompom power sequencing. If more panels are later attached to pompom > > > > then it's fine as long as they work in a compatible way. > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > the timings directly in the device tree file. As I understand it, > > > > trying to describe generic power sequencing in the device tree is > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > per board class) feels like a reasonable compromise. We're not trying > > > > to define fully generic power sequence bindings but we can also take > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > have used the "white lie" approach. One representative panel was > > > > listed in the device tree. The power sequencings of this > > > > representative panel were OK to use across all panels that might be > > > > attached and other differences were handled by EDID. This patch > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > --- > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > hardware definition. It's clearer than the 'white lie' approach. > > > > Yeah, it is a weird grey area between "discoverable" and "not > > discoverable".. but I favor DT reflecting reality as much as > > possible/feasible, so I think this is definity cleaner than "white > > lies" > > This is practically no different than the "white lie". I suppose you > could perhaps call it "more honest", if you want. > > The point remains that unless we describe exactly which panel we're > dealing with, we ultimately have no way of properly quirking anything if > we ever have to. Also, once we allow this kind of wildcard we can > suddenly get into a situation where people might want to reuse this on > something that's not at all a google-pompom board because the same > particular power sequence happens to work on on some other board. > > Similarly I can imagine a situation where we could now have the same > panel supported by multiple different wildcard compatible strings. How > is that supposed to be any cleaner than what we have now? > > And I still keep wondering why bootloaders can't be taught about these > kinds of things. We have in the past discussed various solutions where > the bootloader could detect the type of panel connected and set the > proper compatible string. The bootloader cannot detect the panel without powering up the panel, which it normally does not do if you are not in dev-mode (it would add a significant amount of time to bootup, which is why we can't do this) BR, -R > If that's too complicated and these really are standardized interfaces > that work across a wide range of devices with perhaps a couple of > standard parameter, then introducing a standard connector type like > Rob Herring is suggesting makes more sense because that more properly > describes where exactly the standardization is going on (i.e. at the > interface level rather than the panel level). > > Thierry ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-26 15:18 ` Rob Clark 0 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-26 15:18 UTC (permalink / raw) To: Thierry Reding Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, Linux Kernel Mailing List, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Bjorn Andersson, Rob Herring, Matthias Kaehlcke, Andy Gross, Stephen Boyd, Sam Ravnborg, Steev Klimaszewski On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > - KD KD116N21-30NV-A010 > > > > - KD KD116N09-30NH-A016 > > > > - Starry 2081116HHD028001-51D > > > > - Sharp LQ116M1JW10 > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > panels indicates that they should run with exactly the same timing (to > > > > keep things simple), the 4th panel not only needs different timing but > > > > has a different resolution. > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > we have and all the info needed to drive its pixel clock by reading > > > > the EDID. However, we can do this only after we've powered the panel > > > > on. Powering on the panels requires following the timing diagram in > > > > each panel's datasheet which specifies delays between certain > > > > actions. This means that, while we can be quite dynamic about handling > > > > things we can't just totally skip out on describing the panel like we > > > > could do if it was connected to an external-facing DP port. > > > > > > > > While the different panels have slightly different delays, it's > > > > possible to come up with a set of unified delays that will work on all > > > > the panels. From reading the datasheets: > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > - HPD absent delay: 200 ms > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > * Starry 2081116HHD028001-51D > > > > - HPD absent delay: 100 ms > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > - Unprepare delay: 500 ms > > > > * Sharp LQ116M1JW10 > > > > - HPD absent delay: 200 ms > > > > - Unprepare delay: 500 ms > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > Unified: > > > > - HPD absent delay: 200 ms > > > > - Unprepare delay: 500 ms > > > > - Enable delay: 200 ms > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > as a panel that provides a valid EDID and can work with the standard > > > > pompom power sequencing. If more panels are later attached to pompom > > > > then it's fine as long as they work in a compatible way. > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > the timings directly in the device tree file. As I understand it, > > > > trying to describe generic power sequencing in the device tree is > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > per board class) feels like a reasonable compromise. We're not trying > > > > to define fully generic power sequence bindings but we can also take > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > have used the "white lie" approach. One representative panel was > > > > listed in the device tree. The power sequencings of this > > > > representative panel were OK to use across all panels that might be > > > > attached and other differences were handled by EDID. This patch > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > --- > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > hardware definition. It's clearer than the 'white lie' approach. > > > > Yeah, it is a weird grey area between "discoverable" and "not > > discoverable".. but I favor DT reflecting reality as much as > > possible/feasible, so I think this is definity cleaner than "white > > lies" > > This is practically no different than the "white lie". I suppose you > could perhaps call it "more honest", if you want. > > The point remains that unless we describe exactly which panel we're > dealing with, we ultimately have no way of properly quirking anything if > we ever have to. Also, once we allow this kind of wildcard we can > suddenly get into a situation where people might want to reuse this on > something that's not at all a google-pompom board because the same > particular power sequence happens to work on on some other board. > > Similarly I can imagine a situation where we could now have the same > panel supported by multiple different wildcard compatible strings. How > is that supposed to be any cleaner than what we have now? > > And I still keep wondering why bootloaders can't be taught about these > kinds of things. We have in the past discussed various solutions where > the bootloader could detect the type of panel connected and set the > proper compatible string. The bootloader cannot detect the panel without powering up the panel, which it normally does not do if you are not in dev-mode (it would add a significant amount of time to bootup, which is why we can't do this) BR, -R > If that's too complicated and these really are standardized interfaces > that work across a wide range of devices with perhaps a couple of > standard parameter, then introducing a standard connector type like > Rob Herring is suggesting makes more sense because that more properly > describes where exactly the standardization is going on (i.e. at the > interface level rather than the panel level). > > Thierry _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 15:18 ` Rob Clark @ 2021-03-26 15:24 ` Rob Clark -1 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-26 15:24 UTC (permalink / raw) To: Thierry Reding Cc: Matthias Kaehlcke, Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Rob Herring, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > - KD KD116N21-30NV-A010 > > > > > - KD KD116N09-30NH-A016 > > > > > - Starry 2081116HHD028001-51D > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > panels indicates that they should run with exactly the same timing (to > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > has a different resolution. > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > on. Powering on the panels requires following the timing diagram in > > > > > each panel's datasheet which specifies delays between certain > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > things we can't just totally skip out on describing the panel like we > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > possible to come up with a set of unified delays that will work on all > > > > > the panels. From reading the datasheets: > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > - HPD absent delay: 200 ms > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > * Starry 2081116HHD028001-51D > > > > > - HPD absent delay: 100 ms > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > - Unprepare delay: 500 ms > > > > > * Sharp LQ116M1JW10 > > > > > - HPD absent delay: 200 ms > > > > > - Unprepare delay: 500 ms > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > Unified: > > > > > - HPD absent delay: 200 ms > > > > > - Unprepare delay: 500 ms > > > > > - Enable delay: 200 ms > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > the timings directly in the device tree file. As I understand it, > > > > > trying to describe generic power sequencing in the device tree is > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > to define fully generic power sequence bindings but we can also take > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > have used the "white lie" approach. One representative panel was > > > > > listed in the device tree. The power sequencings of this > > > > > representative panel were OK to use across all panels that might be > > > > > attached and other differences were handled by EDID. This patch > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > --- > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > discoverable".. but I favor DT reflecting reality as much as > > > possible/feasible, so I think this is definity cleaner than "white > > > lies" > > > > This is practically no different than the "white lie". I suppose you > > could perhaps call it "more honest", if you want. > > > > The point remains that unless we describe exactly which panel we're > > dealing with, we ultimately have no way of properly quirking anything if > > we ever have to. Also, once we allow this kind of wildcard we can > > suddenly get into a situation where people might want to reuse this on > > something that's not at all a google-pompom board because the same > > particular power sequence happens to work on on some other board. > > > > Similarly I can imagine a situation where we could now have the same > > panel supported by multiple different wildcard compatible strings. How > > is that supposed to be any cleaner than what we have now? > > > > And I still keep wondering why bootloaders can't be taught about these > > kinds of things. We have in the past discussed various solutions where > > the bootloader could detect the type of panel connected and set the > > proper compatible string. > > The bootloader cannot detect the panel without powering up the panel, > which it normally does not do if you are not in dev-mode (it would add > a significant amount of time to bootup, which is why we can't do this) what if we had a sort of multi-choice panel node: panel: panel { compatible = "panel,one-of"; compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", "vendor3,panel-c"; }; The kernel could construct power sequence timings that are the superset of all the possible panels. That seems about as explicit as we could get in this sort of case. BR, -R > BR, > -R > > > If that's too complicated and these really are standardized interfaces > > that work across a wide range of devices with perhaps a couple of > > standard parameter, then introducing a standard connector type like > > Rob Herring is suggesting makes more sense because that more properly > > describes where exactly the standardization is going on (i.e. at the > > interface level rather than the panel level). > > > > Thierry ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-26 15:24 ` Rob Clark 0 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-26 15:24 UTC (permalink / raw) To: Thierry Reding Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, Linux Kernel Mailing List, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Bjorn Andersson, Rob Herring, Matthias Kaehlcke, Andy Gross, Stephen Boyd, Sam Ravnborg, Steev Klimaszewski On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > - KD KD116N21-30NV-A010 > > > > > - KD KD116N09-30NH-A016 > > > > > - Starry 2081116HHD028001-51D > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > panels indicates that they should run with exactly the same timing (to > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > has a different resolution. > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > on. Powering on the panels requires following the timing diagram in > > > > > each panel's datasheet which specifies delays between certain > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > things we can't just totally skip out on describing the panel like we > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > possible to come up with a set of unified delays that will work on all > > > > > the panels. From reading the datasheets: > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > - HPD absent delay: 200 ms > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > * Starry 2081116HHD028001-51D > > > > > - HPD absent delay: 100 ms > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > - Unprepare delay: 500 ms > > > > > * Sharp LQ116M1JW10 > > > > > - HPD absent delay: 200 ms > > > > > - Unprepare delay: 500 ms > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > Unified: > > > > > - HPD absent delay: 200 ms > > > > > - Unprepare delay: 500 ms > > > > > - Enable delay: 200 ms > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > the timings directly in the device tree file. As I understand it, > > > > > trying to describe generic power sequencing in the device tree is > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > to define fully generic power sequence bindings but we can also take > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > have used the "white lie" approach. One representative panel was > > > > > listed in the device tree. The power sequencings of this > > > > > representative panel were OK to use across all panels that might be > > > > > attached and other differences were handled by EDID. This patch > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > --- > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > discoverable".. but I favor DT reflecting reality as much as > > > possible/feasible, so I think this is definity cleaner than "white > > > lies" > > > > This is practically no different than the "white lie". I suppose you > > could perhaps call it "more honest", if you want. > > > > The point remains that unless we describe exactly which panel we're > > dealing with, we ultimately have no way of properly quirking anything if > > we ever have to. Also, once we allow this kind of wildcard we can > > suddenly get into a situation where people might want to reuse this on > > something that's not at all a google-pompom board because the same > > particular power sequence happens to work on on some other board. > > > > Similarly I can imagine a situation where we could now have the same > > panel supported by multiple different wildcard compatible strings. How > > is that supposed to be any cleaner than what we have now? > > > > And I still keep wondering why bootloaders can't be taught about these > > kinds of things. We have in the past discussed various solutions where > > the bootloader could detect the type of panel connected and set the > > proper compatible string. > > The bootloader cannot detect the panel without powering up the panel, > which it normally does not do if you are not in dev-mode (it would add > a significant amount of time to bootup, which is why we can't do this) what if we had a sort of multi-choice panel node: panel: panel { compatible = "panel,one-of"; compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", "vendor3,panel-c"; }; The kernel could construct power sequence timings that are the superset of all the possible panels. That seems about as explicit as we could get in this sort of case. BR, -R > BR, > -R > > > If that's too complicated and these really are standardized interfaces > > that work across a wide range of devices with perhaps a couple of > > standard parameter, then introducing a standard connector type like > > Rob Herring is suggesting makes more sense because that more properly > > describes where exactly the standardization is going on (i.e. at the > > interface level rather than the panel level). > > > > Thierry _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 15:24 ` Rob Clark @ 2021-03-26 17:00 ` Bjorn Andersson -1 siblings, 0 replies; 40+ messages in thread From: Bjorn Andersson @ 2021-03-26 17:00 UTC (permalink / raw) To: Rob Clark Cc: Thierry Reding, Matthias Kaehlcke, Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Rob Herring, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Fri 26 Mar 10:24 CDT 2021, Rob Clark wrote: > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > - KD KD116N21-30NV-A010 > > > > > > - KD KD116N09-30NH-A016 > > > > > > - Starry 2081116HHD028001-51D > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > has a different resolution. > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > each panel's datasheet which specifies delays between certain > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > the panels. From reading the datasheets: > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > * Starry 2081116HHD028001-51D > > > > > > - HPD absent delay: 100 ms > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > * Sharp LQ116M1JW10 > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > Unified: > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > have used the "white lie" approach. One representative panel was > > > > > > listed in the device tree. The power sequencings of this > > > > > > representative panel were OK to use across all panels that might be > > > > > > attached and other differences were handled by EDID. This patch > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > --- > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > discoverable".. but I favor DT reflecting reality as much as > > > > possible/feasible, so I think this is definity cleaner than "white > > > > lies" > > > > > > This is practically no different than the "white lie". I suppose you > > > could perhaps call it "more honest", if you want. > > > > > > The point remains that unless we describe exactly which panel we're > > > dealing with, we ultimately have no way of properly quirking anything if > > > we ever have to. Also, once we allow this kind of wildcard we can > > > suddenly get into a situation where people might want to reuse this on > > > something that's not at all a google-pompom board because the same > > > particular power sequence happens to work on on some other board. > > > > > > Similarly I can imagine a situation where we could now have the same > > > panel supported by multiple different wildcard compatible strings. How > > > is that supposed to be any cleaner than what we have now? > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > kinds of things. We have in the past discussed various solutions where > > > the bootloader could detect the type of panel connected and set the > > > proper compatible string. > > > > The bootloader cannot detect the panel without powering up the panel, > > which it normally does not do if you are not in dev-mode (it would add > > a significant amount of time to bootup, which is why we can't do this) > > what if we had a sort of multi-choice panel node: > > panel: panel { > compatible = "panel,one-of"; > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > "vendor3,panel-c"; > }; > > The kernel could construct power sequence timings that are the > superset of all the possible panels. That seems about as explicit as > we could get in this sort of case. > Being able to express a "panel selector" like this would certainly be helpful in a number of phones, where a set of gpios or adc values are read to determine which panel is actually mounted. This is easier to do in the bootloader than your case, but the bootloaders I've seen doing this have a tendency to come with a dependency on the DT structure - which wouldn't match the upstream approved DT bindings... Regards, Bjorn ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-26 17:00 ` Bjorn Andersson 0 siblings, 0 replies; 40+ messages in thread From: Bjorn Andersson @ 2021-03-26 17:00 UTC (permalink / raw) To: Rob Clark Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, Linux Kernel Mailing List, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Stephen Boyd, Rob Herring, Thierry Reding, Andy Gross, Sam Ravnborg, Matthias Kaehlcke, Steev Klimaszewski On Fri 26 Mar 10:24 CDT 2021, Rob Clark wrote: > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > - KD KD116N21-30NV-A010 > > > > > > - KD KD116N09-30NH-A016 > > > > > > - Starry 2081116HHD028001-51D > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > has a different resolution. > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > each panel's datasheet which specifies delays between certain > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > the panels. From reading the datasheets: > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > * Starry 2081116HHD028001-51D > > > > > > - HPD absent delay: 100 ms > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > * Sharp LQ116M1JW10 > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > Unified: > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > have used the "white lie" approach. One representative panel was > > > > > > listed in the device tree. The power sequencings of this > > > > > > representative panel were OK to use across all panels that might be > > > > > > attached and other differences were handled by EDID. This patch > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > --- > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > discoverable".. but I favor DT reflecting reality as much as > > > > possible/feasible, so I think this is definity cleaner than "white > > > > lies" > > > > > > This is practically no different than the "white lie". I suppose you > > > could perhaps call it "more honest", if you want. > > > > > > The point remains that unless we describe exactly which panel we're > > > dealing with, we ultimately have no way of properly quirking anything if > > > we ever have to. Also, once we allow this kind of wildcard we can > > > suddenly get into a situation where people might want to reuse this on > > > something that's not at all a google-pompom board because the same > > > particular power sequence happens to work on on some other board. > > > > > > Similarly I can imagine a situation where we could now have the same > > > panel supported by multiple different wildcard compatible strings. How > > > is that supposed to be any cleaner than what we have now? > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > kinds of things. We have in the past discussed various solutions where > > > the bootloader could detect the type of panel connected and set the > > > proper compatible string. > > > > The bootloader cannot detect the panel without powering up the panel, > > which it normally does not do if you are not in dev-mode (it would add > > a significant amount of time to bootup, which is why we can't do this) > > what if we had a sort of multi-choice panel node: > > panel: panel { > compatible = "panel,one-of"; > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > "vendor3,panel-c"; > }; > > The kernel could construct power sequence timings that are the > superset of all the possible panels. That seems about as explicit as > we could get in this sort of case. > Being able to express a "panel selector" like this would certainly be helpful in a number of phones, where a set of gpios or adc values are read to determine which panel is actually mounted. This is easier to do in the bootloader than your case, but the bootloaders I've seen doing this have a tendency to come with a dependency on the DT structure - which wouldn't match the upstream approved DT bindings... Regards, Bjorn _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 15:24 ` Rob Clark @ 2021-03-26 19:48 ` Rob Herring -1 siblings, 0 replies; 40+ messages in thread From: Rob Herring @ 2021-03-26 19:48 UTC (permalink / raw) To: Rob Clark Cc: Thierry Reding, Matthias Kaehlcke, Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > - KD KD116N21-30NV-A010 > > > > > > - KD KD116N09-30NH-A016 > > > > > > - Starry 2081116HHD028001-51D > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > has a different resolution. > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > each panel's datasheet which specifies delays between certain > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > the panels. From reading the datasheets: > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > * Starry 2081116HHD028001-51D > > > > > > - HPD absent delay: 100 ms > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > * Sharp LQ116M1JW10 > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > Unified: > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > have used the "white lie" approach. One representative panel was > > > > > > listed in the device tree. The power sequencings of this > > > > > > representative panel were OK to use across all panels that might be > > > > > > attached and other differences were handled by EDID. This patch > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > --- > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > discoverable".. but I favor DT reflecting reality as much as > > > > possible/feasible, so I think this is definity cleaner than "white > > > > lies" > > > > > > This is practically no different than the "white lie". I suppose you > > > could perhaps call it "more honest", if you want. > > > > > > The point remains that unless we describe exactly which panel we're > > > dealing with, we ultimately have no way of properly quirking anything if > > > we ever have to. Also, once we allow this kind of wildcard we can > > > suddenly get into a situation where people might want to reuse this on > > > something that's not at all a google-pompom board because the same > > > particular power sequence happens to work on on some other board. > > > > > > Similarly I can imagine a situation where we could now have the same > > > panel supported by multiple different wildcard compatible strings. How > > > is that supposed to be any cleaner than what we have now? > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > kinds of things. We have in the past discussed various solutions where > > > the bootloader could detect the type of panel connected and set the > > > proper compatible string. > > > > The bootloader cannot detect the panel without powering up the panel, > > which it normally does not do if you are not in dev-mode (it would add > > a significant amount of time to bootup, which is why we can't do this) > > what if we had a sort of multi-choice panel node: > > panel: panel { > compatible = "panel,one-of"; > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > "vendor3,panel-c"; > }; > > The kernel could construct power sequence timings that are the > superset of all the possible panels. That seems about as explicit as > we could get in this sort of case. If we were to go this route, I'm inclined to say just shove all the possible panel compatibles into 'compatible'. That kind of breaks the notion of most specific to least specific. OTOH, it is saying the set of panels are somehow 'compatible' with each other. If there's not some level of compatibility between the panels, then it's still the bootloader's problem. Rob ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-26 19:48 ` Rob Herring 0 siblings, 0 replies; 40+ messages in thread From: Rob Herring @ 2021-03-26 19:48 UTC (permalink / raw) To: Rob Clark Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, Linux Kernel Mailing List, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Bjorn Andersson, Thierry Reding, Andy Gross, Stephen Boyd, Sam Ravnborg, Matthias Kaehlcke, Steev Klimaszewski On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > - KD KD116N21-30NV-A010 > > > > > > - KD KD116N09-30NH-A016 > > > > > > - Starry 2081116HHD028001-51D > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > has a different resolution. > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > each panel's datasheet which specifies delays between certain > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > the panels. From reading the datasheets: > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > * Starry 2081116HHD028001-51D > > > > > > - HPD absent delay: 100 ms > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > * Sharp LQ116M1JW10 > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > Unified: > > > > > > - HPD absent delay: 200 ms > > > > > > - Unprepare delay: 500 ms > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > have used the "white lie" approach. One representative panel was > > > > > > listed in the device tree. The power sequencings of this > > > > > > representative panel were OK to use across all panels that might be > > > > > > attached and other differences were handled by EDID. This patch > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > --- > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > discoverable".. but I favor DT reflecting reality as much as > > > > possible/feasible, so I think this is definity cleaner than "white > > > > lies" > > > > > > This is practically no different than the "white lie". I suppose you > > > could perhaps call it "more honest", if you want. > > > > > > The point remains that unless we describe exactly which panel we're > > > dealing with, we ultimately have no way of properly quirking anything if > > > we ever have to. Also, once we allow this kind of wildcard we can > > > suddenly get into a situation where people might want to reuse this on > > > something that's not at all a google-pompom board because the same > > > particular power sequence happens to work on on some other board. > > > > > > Similarly I can imagine a situation where we could now have the same > > > panel supported by multiple different wildcard compatible strings. How > > > is that supposed to be any cleaner than what we have now? > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > kinds of things. We have in the past discussed various solutions where > > > the bootloader could detect the type of panel connected and set the > > > proper compatible string. > > > > The bootloader cannot detect the panel without powering up the panel, > > which it normally does not do if you are not in dev-mode (it would add > > a significant amount of time to bootup, which is why we can't do this) > > what if we had a sort of multi-choice panel node: > > panel: panel { > compatible = "panel,one-of"; > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > "vendor3,panel-c"; > }; > > The kernel could construct power sequence timings that are the > superset of all the possible panels. That seems about as explicit as > we could get in this sort of case. If we were to go this route, I'm inclined to say just shove all the possible panel compatibles into 'compatible'. That kind of breaks the notion of most specific to least specific. OTOH, it is saying the set of panels are somehow 'compatible' with each other. If there's not some level of compatibility between the panels, then it's still the bootloader's problem. Rob _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 19:48 ` Rob Herring @ 2021-03-26 22:16 ` Rob Clark -1 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-26 22:16 UTC (permalink / raw) To: Rob Herring Cc: Thierry Reding, Matthias Kaehlcke, Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > has a different resolution. > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > the panels. From reading the datasheets: > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > - HPD absent delay: 100 ms > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > * Sharp LQ116M1JW10 > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > Unified: > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > --- > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > lies" > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > could perhaps call it "more honest", if you want. > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > suddenly get into a situation where people might want to reuse this on > > > > something that's not at all a google-pompom board because the same > > > > particular power sequence happens to work on on some other board. > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > panel supported by multiple different wildcard compatible strings. How > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > kinds of things. We have in the past discussed various solutions where > > > > the bootloader could detect the type of panel connected and set the > > > > proper compatible string. > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > which it normally does not do if you are not in dev-mode (it would add > > > a significant amount of time to bootup, which is why we can't do this) > > > > what if we had a sort of multi-choice panel node: > > > > panel: panel { > > compatible = "panel,one-of"; > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > "vendor3,panel-c"; > > }; > > > > The kernel could construct power sequence timings that are the > > superset of all the possible panels. That seems about as explicit as > > we could get in this sort of case. > > If we were to go this route, I'm inclined to say just shove all the > possible panel compatibles into 'compatible'. That kind of breaks the > notion of most specific to least specific. OTOH, it is saying the set > of panels are somehow 'compatible' with each other. > > If there's not some level of compatibility between the panels, then > it's still the bootloader's problem. > I'm not sure about this.. since there could be slight differences in various delay params between the possible panels. I'd prefer that in panel-simple.c, we listed exact delay params "vendorFoo,panelBar", but it could mean that for a device that had three possible panels the worst case (max of all possible delays) could be higher than any individual choice.. and I don't think we should encourage the "white lie" approach (which will be the obvious outcome of not handling this directly in dt IME, based on prior art). OTOH pushing it to the bootloader, when the bootloader actually has to power up the panel (and abide by the necessary delays) to figure out what choice we have isn't a viable option either. It is better to be explicit about what we know and at the same time about what we don't know. BR, -R ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-26 22:16 ` Rob Clark 0 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-26 22:16 UTC (permalink / raw) To: Rob Herring Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, Linux Kernel Mailing List, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Bjorn Andersson, Thierry Reding, Andy Gross, Stephen Boyd, Sam Ravnborg, Matthias Kaehlcke, Steev Klimaszewski On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > has a different resolution. > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > the panels. From reading the datasheets: > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > - HPD absent delay: 100 ms > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > * Sharp LQ116M1JW10 > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > Unified: > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > --- > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > lies" > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > could perhaps call it "more honest", if you want. > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > suddenly get into a situation where people might want to reuse this on > > > > something that's not at all a google-pompom board because the same > > > > particular power sequence happens to work on on some other board. > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > panel supported by multiple different wildcard compatible strings. How > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > kinds of things. We have in the past discussed various solutions where > > > > the bootloader could detect the type of panel connected and set the > > > > proper compatible string. > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > which it normally does not do if you are not in dev-mode (it would add > > > a significant amount of time to bootup, which is why we can't do this) > > > > what if we had a sort of multi-choice panel node: > > > > panel: panel { > > compatible = "panel,one-of"; > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > "vendor3,panel-c"; > > }; > > > > The kernel could construct power sequence timings that are the > > superset of all the possible panels. That seems about as explicit as > > we could get in this sort of case. > > If we were to go this route, I'm inclined to say just shove all the > possible panel compatibles into 'compatible'. That kind of breaks the > notion of most specific to least specific. OTOH, it is saying the set > of panels are somehow 'compatible' with each other. > > If there's not some level of compatibility between the panels, then > it's still the bootloader's problem. > I'm not sure about this.. since there could be slight differences in various delay params between the possible panels. I'd prefer that in panel-simple.c, we listed exact delay params "vendorFoo,panelBar", but it could mean that for a device that had three possible panels the worst case (max of all possible delays) could be higher than any individual choice.. and I don't think we should encourage the "white lie" approach (which will be the obvious outcome of not handling this directly in dt IME, based on prior art). OTOH pushing it to the bootloader, when the bootloader actually has to power up the panel (and abide by the necessary delays) to figure out what choice we have isn't a viable option either. It is better to be explicit about what we know and at the same time about what we don't know. BR, -R _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 22:16 ` Rob Clark @ 2021-03-26 23:48 ` Rob Herring -1 siblings, 0 replies; 40+ messages in thread From: Rob Herring @ 2021-03-26 23:48 UTC (permalink / raw) To: Rob Clark Cc: Thierry Reding, Matthias Kaehlcke, Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Fri, Mar 26, 2021 at 4:13 PM Rob Clark <robdclark@gmail.com> wrote: > > On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > > has a different resolution. > > > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > > the panels. From reading the datasheets: > > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > > - HPD absent delay: 100 ms > > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > * Sharp LQ116M1JW10 > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > > > Unified: > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > > --- > > > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > > lies" > > > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > > could perhaps call it "more honest", if you want. > > > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > > suddenly get into a situation where people might want to reuse this on > > > > > something that's not at all a google-pompom board because the same > > > > > particular power sequence happens to work on on some other board. > > > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > > panel supported by multiple different wildcard compatible strings. How > > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > > kinds of things. We have in the past discussed various solutions where > > > > > the bootloader could detect the type of panel connected and set the > > > > > proper compatible string. > > > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > > which it normally does not do if you are not in dev-mode (it would add > > > > a significant amount of time to bootup, which is why we can't do this) > > > > > > what if we had a sort of multi-choice panel node: > > > > > > panel: panel { > > > compatible = "panel,one-of"; > > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > > "vendor3,panel-c"; > > > }; > > > > > > The kernel could construct power sequence timings that are the > > > superset of all the possible panels. That seems about as explicit as > > > we could get in this sort of case. > > > > If we were to go this route, I'm inclined to say just shove all the > > possible panel compatibles into 'compatible'. That kind of breaks the > > notion of most specific to least specific. OTOH, it is saying the set > > of panels are somehow 'compatible' with each other. > > > > If there's not some level of compatibility between the panels, then > > it's still the bootloader's problem. > > > > I'm not sure about this.. since there could be slight differences in > various delay params between the possible panels. I'd prefer that in > panel-simple.c, we listed exact delay params "vendorFoo,panelBar", but > it could mean that for a device that had three possible panels the > worst case (max of all possible delays) could be higher than any > individual choice.. and I don't think we should encourage the "white > lie" approach (which will be the obvious outcome of not handling this > directly in dt IME, based on prior art). OTOH pushing it to the > bootloader, when the bootloader actually has to power up the panel > (and abide by the necessary delays) to figure out what choice we have > isn't a viable option either. I was only saying if the panels are different enough and there's not a worse case setting, then it's back to a bootloader problem. If we have multiple distinct compatibles, then it means the kernel should be able to figure out settings that work on all the possible panels listed. > It is better to be explicit about what we know and at the same time > about what we don't know. Can you be explicit about what we know and don't know here? With what you proposed and my alternative, at the end of the day we just have a list of compatibles. The only implicit part is the expectation that the set is somehow compatible with each other. Rob ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-26 23:48 ` Rob Herring 0 siblings, 0 replies; 40+ messages in thread From: Rob Herring @ 2021-03-26 23:48 UTC (permalink / raw) To: Rob Clark Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, Linux Kernel Mailing List, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Bjorn Andersson, Thierry Reding, Andy Gross, Stephen Boyd, Sam Ravnborg, Matthias Kaehlcke, Steev Klimaszewski On Fri, Mar 26, 2021 at 4:13 PM Rob Clark <robdclark@gmail.com> wrote: > > On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > > has a different resolution. > > > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > > the panels. From reading the datasheets: > > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > > - HPD absent delay: 100 ms > > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > * Sharp LQ116M1JW10 > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > > > Unified: > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > > --- > > > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > > lies" > > > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > > could perhaps call it "more honest", if you want. > > > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > > suddenly get into a situation where people might want to reuse this on > > > > > something that's not at all a google-pompom board because the same > > > > > particular power sequence happens to work on on some other board. > > > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > > panel supported by multiple different wildcard compatible strings. How > > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > > kinds of things. We have in the past discussed various solutions where > > > > > the bootloader could detect the type of panel connected and set the > > > > > proper compatible string. > > > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > > which it normally does not do if you are not in dev-mode (it would add > > > > a significant amount of time to bootup, which is why we can't do this) > > > > > > what if we had a sort of multi-choice panel node: > > > > > > panel: panel { > > > compatible = "panel,one-of"; > > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > > "vendor3,panel-c"; > > > }; > > > > > > The kernel could construct power sequence timings that are the > > > superset of all the possible panels. That seems about as explicit as > > > we could get in this sort of case. > > > > If we were to go this route, I'm inclined to say just shove all the > > possible panel compatibles into 'compatible'. That kind of breaks the > > notion of most specific to least specific. OTOH, it is saying the set > > of panels are somehow 'compatible' with each other. > > > > If there's not some level of compatibility between the panels, then > > it's still the bootloader's problem. > > > > I'm not sure about this.. since there could be slight differences in > various delay params between the possible panels. I'd prefer that in > panel-simple.c, we listed exact delay params "vendorFoo,panelBar", but > it could mean that for a device that had three possible panels the > worst case (max of all possible delays) could be higher than any > individual choice.. and I don't think we should encourage the "white > lie" approach (which will be the obvious outcome of not handling this > directly in dt IME, based on prior art). OTOH pushing it to the > bootloader, when the bootloader actually has to power up the panel > (and abide by the necessary delays) to figure out what choice we have > isn't a viable option either. I was only saying if the panels are different enough and there's not a worse case setting, then it's back to a bootloader problem. If we have multiple distinct compatibles, then it means the kernel should be able to figure out settings that work on all the possible panels listed. > It is better to be explicit about what we know and at the same time > about what we don't know. Can you be explicit about what we know and don't know here? With what you proposed and my alternative, at the end of the day we just have a list of compatibles. The only implicit part is the expectation that the set is somehow compatible with each other. Rob _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 23:48 ` Rob Herring @ 2021-03-27 0:33 ` Rob Clark -1 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-27 0:33 UTC (permalink / raw) To: Rob Herring Cc: Thierry Reding, Matthias Kaehlcke, Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Fri, Mar 26, 2021 at 4:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > On Fri, Mar 26, 2021 at 4:13 PM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > > > has a different resolution. > > > > > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > > > the panels. From reading the datasheets: > > > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > > > - HPD absent delay: 100 ms > > > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > * Sharp LQ116M1JW10 > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > > > > > Unified: > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > > > --- > > > > > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > > > lies" > > > > > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > > > could perhaps call it "more honest", if you want. > > > > > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > > > suddenly get into a situation where people might want to reuse this on > > > > > > something that's not at all a google-pompom board because the same > > > > > > particular power sequence happens to work on on some other board. > > > > > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > > > panel supported by multiple different wildcard compatible strings. How > > > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > > > kinds of things. We have in the past discussed various solutions where > > > > > > the bootloader could detect the type of panel connected and set the > > > > > > proper compatible string. > > > > > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > > > which it normally does not do if you are not in dev-mode (it would add > > > > > a significant amount of time to bootup, which is why we can't do this) > > > > > > > > what if we had a sort of multi-choice panel node: > > > > > > > > panel: panel { > > > > compatible = "panel,one-of"; > > > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > > > "vendor3,panel-c"; > > > > }; > > > > > > > > The kernel could construct power sequence timings that are the > > > > superset of all the possible panels. That seems about as explicit as > > > > we could get in this sort of case. > > > > > > If we were to go this route, I'm inclined to say just shove all the > > > possible panel compatibles into 'compatible'. That kind of breaks the > > > notion of most specific to least specific. OTOH, it is saying the set > > > of panels are somehow 'compatible' with each other. > > > > > > If there's not some level of compatibility between the panels, then > > > it's still the bootloader's problem. > > > > > > > I'm not sure about this.. since there could be slight differences in > > various delay params between the possible panels. I'd prefer that in > > panel-simple.c, we listed exact delay params "vendorFoo,panelBar", but > > it could mean that for a device that had three possible panels the > > worst case (max of all possible delays) could be higher than any > > individual choice.. and I don't think we should encourage the "white > > lie" approach (which will be the obvious outcome of not handling this > > directly in dt IME, based on prior art). OTOH pushing it to the > > bootloader, when the bootloader actually has to power up the panel > > (and abide by the necessary delays) to figure out what choice we have > > isn't a viable option either. > > I was only saying if the panels are different enough and there's not a > worse case setting, then it's back to a bootloader problem. If we have > multiple distinct compatibles, then it means the kernel should be able > to figure out settings that work on all the possible panels listed. > > > It is better to be explicit about what we know and at the same time > > about what we don't know. > > Can you be explicit about what we know and don't know here? With what > you proposed and my alternative, at the end of the day we just have a > list of compatibles. The only implicit part is the expectation that > the set is somehow compatible with each other. > Ok, I think I was being incompatible with my definition of "compatible" ;-) To make sure we are on the same page, this is what I have in mind: 1) the panels are compatible enough that if a user breaks their panel and takes device in for repair, they might end up with a different panel 2) the different possible panels may have different power-on delay, etc, but max of all possible power on delays is fine and enough to get the kernel to the point that it can probe EDID and figure out the rest BR, -R ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-27 0:33 ` Rob Clark 0 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-27 0:33 UTC (permalink / raw) To: Rob Herring Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, Linux Kernel Mailing List, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Bjorn Andersson, Thierry Reding, Andy Gross, Stephen Boyd, Sam Ravnborg, Matthias Kaehlcke, Steev Klimaszewski On Fri, Mar 26, 2021 at 4:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > On Fri, Mar 26, 2021 at 4:13 PM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > > > has a different resolution. > > > > > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > > > the panels. From reading the datasheets: > > > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > > > - HPD absent delay: 100 ms > > > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > * Sharp LQ116M1JW10 > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > > > > > Unified: > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > > > --- > > > > > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > > > lies" > > > > > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > > > could perhaps call it "more honest", if you want. > > > > > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > > > suddenly get into a situation where people might want to reuse this on > > > > > > something that's not at all a google-pompom board because the same > > > > > > particular power sequence happens to work on on some other board. > > > > > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > > > panel supported by multiple different wildcard compatible strings. How > > > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > > > kinds of things. We have in the past discussed various solutions where > > > > > > the bootloader could detect the type of panel connected and set the > > > > > > proper compatible string. > > > > > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > > > which it normally does not do if you are not in dev-mode (it would add > > > > > a significant amount of time to bootup, which is why we can't do this) > > > > > > > > what if we had a sort of multi-choice panel node: > > > > > > > > panel: panel { > > > > compatible = "panel,one-of"; > > > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > > > "vendor3,panel-c"; > > > > }; > > > > > > > > The kernel could construct power sequence timings that are the > > > > superset of all the possible panels. That seems about as explicit as > > > > we could get in this sort of case. > > > > > > If we were to go this route, I'm inclined to say just shove all the > > > possible panel compatibles into 'compatible'. That kind of breaks the > > > notion of most specific to least specific. OTOH, it is saying the set > > > of panels are somehow 'compatible' with each other. > > > > > > If there's not some level of compatibility between the panels, then > > > it's still the bootloader's problem. > > > > > > > I'm not sure about this.. since there could be slight differences in > > various delay params between the possible panels. I'd prefer that in > > panel-simple.c, we listed exact delay params "vendorFoo,panelBar", but > > it could mean that for a device that had three possible panels the > > worst case (max of all possible delays) could be higher than any > > individual choice.. and I don't think we should encourage the "white > > lie" approach (which will be the obvious outcome of not handling this > > directly in dt IME, based on prior art). OTOH pushing it to the > > bootloader, when the bootloader actually has to power up the panel > > (and abide by the necessary delays) to figure out what choice we have > > isn't a viable option either. > > I was only saying if the panels are different enough and there's not a > worse case setting, then it's back to a bootloader problem. If we have > multiple distinct compatibles, then it means the kernel should be able > to figure out settings that work on all the possible panels listed. > > > It is better to be explicit about what we know and at the same time > > about what we don't know. > > Can you be explicit about what we know and don't know here? With what > you proposed and my alternative, at the end of the day we just have a > list of compatibles. The only implicit part is the expectation that > the set is somehow compatible with each other. > Ok, I think I was being incompatible with my definition of "compatible" ;-) To make sure we are on the same page, this is what I have in mind: 1) the panels are compatible enough that if a user breaks their panel and takes device in for repair, they might end up with a different panel 2) the different possible panels may have different power-on delay, etc, but max of all possible power on delays is fine and enough to get the kernel to the point that it can probe EDID and figure out the rest BR, -R _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-27 0:33 ` Rob Clark @ 2021-03-27 0:49 ` Rob Clark -1 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-27 0:49 UTC (permalink / raw) To: Rob Herring Cc: Thierry Reding, Matthias Kaehlcke, Douglas Anderson, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List On Fri, Mar 26, 2021 at 5:33 PM Rob Clark <robdclark@gmail.com> wrote: > > On Fri, Mar 26, 2021 at 4:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Fri, Mar 26, 2021 at 4:13 PM Rob Clark <robdclark@gmail.com> wrote: > > > > > > On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > > > > has a different resolution. > > > > > > > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > > > > the panels. From reading the datasheets: > > > > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > > > > - HPD absent delay: 100 ms > > > > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > > * Sharp LQ116M1JW10 > > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > > > > > > > Unified: > > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > > > > lies" > > > > > > > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > > > > could perhaps call it "more honest", if you want. > > > > > > > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > > > > suddenly get into a situation where people might want to reuse this on > > > > > > > something that's not at all a google-pompom board because the same > > > > > > > particular power sequence happens to work on on some other board. > > > > > > > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > > > > panel supported by multiple different wildcard compatible strings. How > > > > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > > > > kinds of things. We have in the past discussed various solutions where > > > > > > > the bootloader could detect the type of panel connected and set the > > > > > > > proper compatible string. > > > > > > > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > > > > which it normally does not do if you are not in dev-mode (it would add > > > > > > a significant amount of time to bootup, which is why we can't do this) > > > > > > > > > > what if we had a sort of multi-choice panel node: > > > > > > > > > > panel: panel { > > > > > compatible = "panel,one-of"; > > > > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > > > > "vendor3,panel-c"; > > > > > }; > > > > > > > > > > The kernel could construct power sequence timings that are the > > > > > superset of all the possible panels. That seems about as explicit as > > > > > we could get in this sort of case. > > > > > > > > If we were to go this route, I'm inclined to say just shove all the > > > > possible panel compatibles into 'compatible'. That kind of breaks the > > > > notion of most specific to least specific. OTOH, it is saying the set > > > > of panels are somehow 'compatible' with each other. > > > > > > > > If there's not some level of compatibility between the panels, then > > > > it's still the bootloader's problem. > > > > > > > > > > I'm not sure about this.. since there could be slight differences in > > > various delay params between the possible panels. I'd prefer that in > > > panel-simple.c, we listed exact delay params "vendorFoo,panelBar", but > > > it could mean that for a device that had three possible panels the > > > worst case (max of all possible delays) could be higher than any > > > individual choice.. and I don't think we should encourage the "white > > > lie" approach (which will be the obvious outcome of not handling this > > > directly in dt IME, based on prior art). OTOH pushing it to the > > > bootloader, when the bootloader actually has to power up the panel > > > (and abide by the necessary delays) to figure out what choice we have > > > isn't a viable option either. > > > > I was only saying if the panels are different enough and there's not a > > worse case setting, then it's back to a bootloader problem. If we have > > multiple distinct compatibles, then it means the kernel should be able > > to figure out settings that work on all the possible panels listed. > > > > > It is better to be explicit about what we know and at the same time > > > about what we don't know. > > > > Can you be explicit about what we know and don't know here? With what > > you proposed and my alternative, at the end of the day we just have a > > list of compatibles. The only implicit part is the expectation that > > the set is somehow compatible with each other. > > > > Ok, I think I was being incompatible with my definition of "compatible" ;-) > > To make sure we are on the same page, this is what I have in mind: > > 1) the panels are compatible enough that if a user breaks their panel > and takes device in for repair, they might end up with a different > panel > 2) the different possible panels may have different power-on delay, > etc, but max of all possible power on delays is fine and enough to get > the kernel to the point that it can probe EDID and figure out the rest > (by which I mean, I think that is what you are saying.. earlier I was considering panel A, B, and C as not being compatible if they had different power on delays) BR, -R ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-27 0:49 ` Rob Clark 0 siblings, 0 replies; 40+ messages in thread From: Rob Clark @ 2021-03-27 0:49 UTC (permalink / raw) To: Rob Herring Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, Linux Kernel Mailing List, David Airlie, linux-arm-msm, Douglas Anderson, dri-devel, Bjorn Andersson, Thierry Reding, Andy Gross, Stephen Boyd, Sam Ravnborg, Matthias Kaehlcke, Steev Klimaszewski On Fri, Mar 26, 2021 at 5:33 PM Rob Clark <robdclark@gmail.com> wrote: > > On Fri, Mar 26, 2021 at 4:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Fri, Mar 26, 2021 at 4:13 PM Rob Clark <robdclark@gmail.com> wrote: > > > > > > On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > > > > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > > > > has a different resolution. > > > > > > > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > > > > the panels. From reading the datasheets: > > > > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > > > > - HPD absent delay: 100 ms > > > > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > > * Sharp LQ116M1JW10 > > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > > > > > > > Unified: > > > > > > > > > > - HPD absent delay: 200 ms > > > > > > > > > > - Unprepare delay: 500 ms > > > > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > > > > lies" > > > > > > > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > > > > could perhaps call it "more honest", if you want. > > > > > > > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > > > > suddenly get into a situation where people might want to reuse this on > > > > > > > something that's not at all a google-pompom board because the same > > > > > > > particular power sequence happens to work on on some other board. > > > > > > > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > > > > panel supported by multiple different wildcard compatible strings. How > > > > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > > > > kinds of things. We have in the past discussed various solutions where > > > > > > > the bootloader could detect the type of panel connected and set the > > > > > > > proper compatible string. > > > > > > > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > > > > which it normally does not do if you are not in dev-mode (it would add > > > > > > a significant amount of time to bootup, which is why we can't do this) > > > > > > > > > > what if we had a sort of multi-choice panel node: > > > > > > > > > > panel: panel { > > > > > compatible = "panel,one-of"; > > > > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > > > > "vendor3,panel-c"; > > > > > }; > > > > > > > > > > The kernel could construct power sequence timings that are the > > > > > superset of all the possible panels. That seems about as explicit as > > > > > we could get in this sort of case. > > > > > > > > If we were to go this route, I'm inclined to say just shove all the > > > > possible panel compatibles into 'compatible'. That kind of breaks the > > > > notion of most specific to least specific. OTOH, it is saying the set > > > > of panels are somehow 'compatible' with each other. > > > > > > > > If there's not some level of compatibility between the panels, then > > > > it's still the bootloader's problem. > > > > > > > > > > I'm not sure about this.. since there could be slight differences in > > > various delay params between the possible panels. I'd prefer that in > > > panel-simple.c, we listed exact delay params "vendorFoo,panelBar", but > > > it could mean that for a device that had three possible panels the > > > worst case (max of all possible delays) could be higher than any > > > individual choice.. and I don't think we should encourage the "white > > > lie" approach (which will be the obvious outcome of not handling this > > > directly in dt IME, based on prior art). OTOH pushing it to the > > > bootloader, when the bootloader actually has to power up the panel > > > (and abide by the necessary delays) to figure out what choice we have > > > isn't a viable option either. > > > > I was only saying if the panels are different enough and there's not a > > worse case setting, then it's back to a bootloader problem. If we have > > multiple distinct compatibles, then it means the kernel should be able > > to figure out settings that work on all the possible panels listed. > > > > > It is better to be explicit about what we know and at the same time > > > about what we don't know. > > > > Can you be explicit about what we know and don't know here? With what > > you proposed and my alternative, at the end of the day we just have a > > list of compatibles. The only implicit part is the expectation that > > the set is somehow compatible with each other. > > > > Ok, I think I was being incompatible with my definition of "compatible" ;-) > > To make sure we are on the same page, this is what I have in mind: > > 1) the panels are compatible enough that if a user breaks their panel > and takes device in for repair, they might end up with a different > panel > 2) the different possible panels may have different power-on delay, > etc, but max of all possible power on delays is fine and enough to get > the kernel to the point that it can probe EDID and figure out the rest > (by which I mean, I think that is what you are saying.. earlier I was considering panel A, B, and C as not being compatible if they had different power on delays) BR, -R _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 19:48 ` Rob Herring @ 2021-03-29 15:51 ` Doug Anderson -1 siblings, 0 replies; 40+ messages in thread From: Doug Anderson @ 2021-03-29 15:51 UTC (permalink / raw) To: Rob Herring Cc: Rob Clark, Thierry Reding, Matthias Kaehlcke, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List Hi, On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > has a different resolution. > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > the panels. From reading the datasheets: > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > - HPD absent delay: 100 ms > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > * Sharp LQ116M1JW10 > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > Unified: > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > --- > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > lies" > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > could perhaps call it "more honest", if you want. > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > suddenly get into a situation where people might want to reuse this on > > > > something that's not at all a google-pompom board because the same > > > > particular power sequence happens to work on on some other board. > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > panel supported by multiple different wildcard compatible strings. How > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > kinds of things. We have in the past discussed various solutions where > > > > the bootloader could detect the type of panel connected and set the > > > > proper compatible string. > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > which it normally does not do if you are not in dev-mode (it would add > > > a significant amount of time to bootup, which is why we can't do this) > > > > what if we had a sort of multi-choice panel node: > > > > panel: panel { > > compatible = "panel,one-of"; > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > "vendor3,panel-c"; > > }; > > > > The kernel could construct power sequence timings that are the > > superset of all the possible panels. That seems about as explicit as > > we could get in this sort of case. > > If we were to go this route, I'm inclined to say just shove all the > possible panel compatibles into 'compatible'. That kind of breaks the > notion of most specific to least specific. OTOH, it is saying the set > of panels are somehow 'compatible' with each other. > > If there's not some level of compatibility between the panels, then > it's still the bootloader's problem. I guess I missed all the fun! OK, so I think the approach is: 1. List all the possible panels straight in the "compatible" string in arbitrary order. 2. We'll change the interpretation of "compatible" but just for panels. 2a) Normal interpretation of "compatible" (from devicetree spec) is most specific to least specific. We look at the first "compatible" string. If we have a driver for that specific device, use it. If not then move on to the next "compatible" and look for a driver for it. 2b) Interpretation of "compatible" for panels nodes: an unordered list of possible panels that might be stuffed. In order to be listed like this there must logically be a way to drive things in a way that would be compatible with all possible panels here. Presumably no existing panels are relying on the old interpretation? Personally I like Rob Clark's "panel,one-of" a bit better just because it doesn't require the redefinition of a standard property, but if everyone likes the redefinition then I won't object. -Doug ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-29 15:51 ` Doug Anderson 0 siblings, 0 replies; 40+ messages in thread From: Doug Anderson @ 2021-03-29 15:51 UTC (permalink / raw) To: Rob Herring Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Thierry Reding, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Matthias Kaehlcke, Linux Kernel Mailing List Hi, On Fri, Mar 26, 2021 at 12:48 PM Rob Herring <robh+dt@kernel.org> wrote: > > On Fri, Mar 26, 2021 at 9:20 AM Rob Clark <robdclark@gmail.com> wrote: > > > > On Fri, Mar 26, 2021 at 8:18 AM Rob Clark <robdclark@gmail.com> wrote: > > > > > > On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > > > On Wed, Mar 17, 2021 at 06:53:04PM -0700, Rob Clark wrote: > > > > > On Wed, Mar 17, 2021 at 4:27 PM Matthias Kaehlcke <mka@chromium.org> wrote: > > > > > > > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > > > > - KD KD116N21-30NV-A010 > > > > > > > - KD KD116N09-30NH-A016 > > > > > > > - Starry 2081116HHD028001-51D > > > > > > > - Sharp LQ116M1JW10 > > > > > > > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > > > > panels indicates that they should run with exactly the same timing (to > > > > > > > keep things simple), the 4th panel not only needs different timing but > > > > > > > has a different resolution. > > > > > > > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > > > > we have and all the info needed to drive its pixel clock by reading > > > > > > > the EDID. However, we can do this only after we've powered the panel > > > > > > > on. Powering on the panels requires following the timing diagram in > > > > > > > each panel's datasheet which specifies delays between certain > > > > > > > actions. This means that, while we can be quite dynamic about handling > > > > > > > things we can't just totally skip out on describing the panel like we > > > > > > > could do if it was connected to an external-facing DP port. > > > > > > > > > > > > > > While the different panels have slightly different delays, it's > > > > > > > possible to come up with a set of unified delays that will work on all > > > > > > > the panels. From reading the datasheets: > > > > > > > * KD KD116N21-30NV-A010 and KD KD116N09-30NH-A016 > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 150 ms (datasheet is confusing, might be 500 ms) > > > > > > > * Starry 2081116HHD028001-51D > > > > > > > - HPD absent delay: 100 ms > > > > > > > - Enable delay: (link training done till enable BL): 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > * Sharp LQ116M1JW10 > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > - Prepare to enable delay (power on till backlight): 100 ms > > > > > > > > > > > > > > Unified: > > > > > > > - HPD absent delay: 200 ms > > > > > > > - Unprepare delay: 500 ms > > > > > > > - Enable delay: 200 ms > > > > > > > > > > > > > > NOTE: in theory the only thing that we _really_ need unity on is the > > > > > > > "HPD absent delay" since once the panel asserts HPD we can read the > > > > > > > EDID and could make per-panel decisions if we wanted. > > > > > > > > > > > > > > Let's create a definition of "a panel that can be attached to pompom" > > > > > > > as a panel that provides a valid EDID and can work with the standard > > > > > > > pompom power sequencing. If more panels are later attached to pompom > > > > > > > then it's fine as long as they work in a compatible way. > > > > > > > > > > > > > > One might ask why we can't just use a generic string here and provide > > > > > > > the timings directly in the device tree file. As I understand it, > > > > > > > trying to describe generic power sequencing in the device tree is > > > > > > > frowned upon and the one instance (SD/MMC) is regarded as a mistake > > > > > > > that shouldn't be repeated. Specifying a power sequence per board (or > > > > > > > per board class) feels like a reasonable compromise. We're not trying > > > > > > > to define fully generic power sequence bindings but we can also take > > > > > > > advantage of the semi-probable properties of the attached device. > > > > > > > > > > > > > > NOTE: I believe that past instances of supporting this type of thing > > > > > > > have used the "white lie" approach. One representative panel was > > > > > > > listed in the device tree. The power sequencings of this > > > > > > > representative panel were OK to use across all panels that might be > > > > > > > attached and other differences were handled by EDID. This patch > > > > > > > attempts to set a new precedent and avoid the need for the white lie. > > > > > > > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > > --- > > > > > > > > > > > > Sounds reasonable to me if DT maintainers can live with this abstract > > > > > > hardware definition. It's clearer than the 'white lie' approach. > > > > > > > > > > Yeah, it is a weird grey area between "discoverable" and "not > > > > > discoverable".. but I favor DT reflecting reality as much as > > > > > possible/feasible, so I think this is definity cleaner than "white > > > > > lies" > > > > > > > > This is practically no different than the "white lie". I suppose you > > > > could perhaps call it "more honest", if you want. > > > > > > > > The point remains that unless we describe exactly which panel we're > > > > dealing with, we ultimately have no way of properly quirking anything if > > > > we ever have to. Also, once we allow this kind of wildcard we can > > > > suddenly get into a situation where people might want to reuse this on > > > > something that's not at all a google-pompom board because the same > > > > particular power sequence happens to work on on some other board. > > > > > > > > Similarly I can imagine a situation where we could now have the same > > > > panel supported by multiple different wildcard compatible strings. How > > > > is that supposed to be any cleaner than what we have now? > > > > > > > > And I still keep wondering why bootloaders can't be taught about these > > > > kinds of things. We have in the past discussed various solutions where > > > > the bootloader could detect the type of panel connected and set the > > > > proper compatible string. > > > > > > The bootloader cannot detect the panel without powering up the panel, > > > which it normally does not do if you are not in dev-mode (it would add > > > a significant amount of time to bootup, which is why we can't do this) > > > > what if we had a sort of multi-choice panel node: > > > > panel: panel { > > compatible = "panel,one-of"; > > compatible-one-of = "vendor1,panel-a", "vendor2,panel-b", > > "vendor3,panel-c"; > > }; > > > > The kernel could construct power sequence timings that are the > > superset of all the possible panels. That seems about as explicit as > > we could get in this sort of case. > > If we were to go this route, I'm inclined to say just shove all the > possible panel compatibles into 'compatible'. That kind of breaks the > notion of most specific to least specific. OTOH, it is saying the set > of panels are somehow 'compatible' with each other. > > If there's not some level of compatibility between the panels, then > it's still the bootloader's problem. I guess I missed all the fun! OK, so I think the approach is: 1. List all the possible panels straight in the "compatible" string in arbitrary order. 2. We'll change the interpretation of "compatible" but just for panels. 2a) Normal interpretation of "compatible" (from devicetree spec) is most specific to least specific. We look at the first "compatible" string. If we have a driver for that specific device, use it. If not then move on to the next "compatible" and look for a driver for it. 2b) Interpretation of "compatible" for panels nodes: an unordered list of possible panels that might be stuffed. In order to be listed like this there must logically be a way to drive things in a way that would be compatible with all possible panels here. Presumably no existing panels are relying on the old interpretation? Personally I like Rob Clark's "panel,one-of" a bit better just because it doesn't require the redefinition of a standard property, but if everyone likes the redefinition then I won't object. -Doug _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 12:39 ` Thierry Reding @ 2021-03-29 16:07 ` Doug Anderson -1 siblings, 0 replies; 40+ messages in thread From: Doug Anderson @ 2021-03-29 16:07 UTC (permalink / raw) To: Thierry Reding Cc: Rob Clark, Matthias Kaehlcke, Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Rob Herring, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List Hi, On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > The point remains that unless we describe exactly which panel we're > dealing with, we ultimately have no way of properly quirking anything if > we ever have to. Just to clarify here: with my initial proposal we actually could still quirk things if we had to. If the quirk needed to be applied before power on we'd just have to apply the quirk to the whole board (which we'd have to do anyway). After the panel was powered on then we could read the EDID and apply a quirk based on what the EDID tells us, right? > Also, once we allow this kind of wildcard we can > suddenly get into a situation where people might want to reuse this on > something that's not at all a google-pompom board because the same > particular power sequence happens to work on on some other board. That's a legit concern. Of course, people could already do that with existing panels right? One would also hope that if they reused this they also used the "more specific to least specific" rule, so someone could reuse (without any problems) with: compatible = "some-other-company,some-other-board-panel", "google,pompom-panel" That doesn't seem like it would be terrible. > Similarly I can imagine a situation where we could now have the same > panel supported by multiple different wildcard compatible strings. How > is that supposed to be any cleaner than what we have now? I'm tempted to call this (same panel supported by multiple different compatible strings) a feature, actually. Specifically: Even if the exact same hardware is shipped with more than one board, it may have a different EDID programmed into it. From what I've seen the timings used on a panel may need to be adjusted based on the SoC used (and what clock rates it can provide / features of the underlying display driver), EMI concerns, and power consumption concerns. Once a different EDID is programmed in it then it sorta becomes a "different" panel, right? I think sometimes (?) panel vendors assign a slightly different model number per board, but I'm not sure. -Doug ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-29 16:07 ` Doug Anderson 0 siblings, 0 replies; 40+ messages in thread From: Doug Anderson @ 2021-03-29 16:07 UTC (permalink / raw) To: Thierry Reding Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Rob Herring, Matthias Kaehlcke, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Linux Kernel Mailing List Hi, On Fri, Mar 26, 2021 at 5:38 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > The point remains that unless we describe exactly which panel we're > dealing with, we ultimately have no way of properly quirking anything if > we ever have to. Just to clarify here: with my initial proposal we actually could still quirk things if we had to. If the quirk needed to be applied before power on we'd just have to apply the quirk to the whole board (which we'd have to do anyway). After the panel was powered on then we could read the EDID and apply a quirk based on what the EDID tells us, right? > Also, once we allow this kind of wildcard we can > suddenly get into a situation where people might want to reuse this on > something that's not at all a google-pompom board because the same > particular power sequence happens to work on on some other board. That's a legit concern. Of course, people could already do that with existing panels right? One would also hope that if they reused this they also used the "more specific to least specific" rule, so someone could reuse (without any problems) with: compatible = "some-other-company,some-other-board-panel", "google,pompom-panel" That doesn't seem like it would be terrible. > Similarly I can imagine a situation where we could now have the same > panel supported by multiple different wildcard compatible strings. How > is that supposed to be any cleaner than what we have now? I'm tempted to call this (same panel supported by multiple different compatible strings) a feature, actually. Specifically: Even if the exact same hardware is shipped with more than one board, it may have a different EDID programmed into it. From what I've seen the timings used on a panel may need to be adjusted based on the SoC used (and what clock rates it can provide / features of the underlying display driver), EMI concerns, and power consumption concerns. Once a different EDID is programmed in it then it sorta becomes a "different" panel, right? I think sometimes (?) panel vendors assign a slightly different model number per board, but I'm not sure. -Doug _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-16 21:08 ` Douglas Anderson @ 2021-03-26 0:09 ` Rob Herring -1 siblings, 0 replies; 40+ messages in thread From: Rob Herring @ 2021-03-26 0:09 UTC (permalink / raw) To: Douglas Anderson Cc: Bjorn Andersson, Thierry Reding, Sam Ravnborg, robdclark, Daniel Vetter, dri-devel, David Airlie, devicetree, swboyd, linux-arm-msm, Steev Klimaszewski, eballetbo, Andy Gross, Linus Walleij, drinkcat, linux-kernel On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > The sc7180-trogdor-pompom board might be attached to any number of a > pile of eDP panels. At the moment I'm told that the list might include: > - KD KD116N21-30NV-A010 > - KD KD116N09-30NH-A016 > - Starry 2081116HHD028001-51D > - Sharp LQ116M1JW10 > > It should be noted that while the EDID programmed in the first 3 > panels indicates that they should run with exactly the same timing (to > keep things simple), the 4th panel not only needs different timing but > has a different resolution. > > As is true in general with eDP panels, we can figure out which panel > we have and all the info needed to drive its pixel clock by reading > the EDID. However, we can do this only after we've powered the panel > on. Powering on the panels requires following the timing diagram in > each panel's datasheet which specifies delays between certain > actions. This means that, while we can be quite dynamic about handling > things we can't just totally skip out on describing the panel like we > could do if it was connected to an external-facing DP port. Is this a 'standard' eDP connector? AFAICT, there does seem to be such a thing. I've said in the past I'd be okay with a edp-connector node. If that needs just the "HPD absent delay" property, I think that would be okay. It's just a never ending stream of new properties with each new panel that I don't want to see. Rob ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-26 0:09 ` Rob Herring 0 siblings, 0 replies; 40+ messages in thread From: Rob Herring @ 2021-03-26 0:09 UTC (permalink / raw) To: Douglas Anderson Cc: robdclark, devicetree, drinkcat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Thierry Reding, Steev Klimaszewski, swboyd, Sam Ravnborg, linux-kernel On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > The sc7180-trogdor-pompom board might be attached to any number of a > pile of eDP panels. At the moment I'm told that the list might include: > - KD KD116N21-30NV-A010 > - KD KD116N09-30NH-A016 > - Starry 2081116HHD028001-51D > - Sharp LQ116M1JW10 > > It should be noted that while the EDID programmed in the first 3 > panels indicates that they should run with exactly the same timing (to > keep things simple), the 4th panel not only needs different timing but > has a different resolution. > > As is true in general with eDP panels, we can figure out which panel > we have and all the info needed to drive its pixel clock by reading > the EDID. However, we can do this only after we've powered the panel > on. Powering on the panels requires following the timing diagram in > each panel's datasheet which specifies delays between certain > actions. This means that, while we can be quite dynamic about handling > things we can't just totally skip out on describing the panel like we > could do if it was connected to an external-facing DP port. Is this a 'standard' eDP connector? AFAICT, there does seem to be such a thing. I've said in the past I'd be okay with a edp-connector node. If that needs just the "HPD absent delay" property, I think that would be okay. It's just a never ending stream of new properties with each new panel that I don't want to see. Rob _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-26 0:09 ` Rob Herring @ 2021-03-29 16:25 ` Doug Anderson -1 siblings, 0 replies; 40+ messages in thread From: Doug Anderson @ 2021-03-29 16:25 UTC (permalink / raw) To: Rob Herring Cc: Bjorn Andersson, Thierry Reding, Sam Ravnborg, Rob Clark, Daniel Vetter, dri-devel, David Airlie, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Stephen Boyd, linux-arm-msm, Steev Klimaszewski, Enric Balletbo Serra, Andy Gross, Linus Walleij, Nicolas Boichat, LKML Hi, On Thu, Mar 25, 2021 at 5:09 PM Rob Herring <robh@kernel.org> wrote: > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > The sc7180-trogdor-pompom board might be attached to any number of a > > pile of eDP panels. At the moment I'm told that the list might include: > > - KD KD116N21-30NV-A010 > > - KD KD116N09-30NH-A016 > > - Starry 2081116HHD028001-51D > > - Sharp LQ116M1JW10 > > > > It should be noted that while the EDID programmed in the first 3 > > panels indicates that they should run with exactly the same timing (to > > keep things simple), the 4th panel not only needs different timing but > > has a different resolution. > > > > As is true in general with eDP panels, we can figure out which panel > > we have and all the info needed to drive its pixel clock by reading > > the EDID. However, we can do this only after we've powered the panel > > on. Powering on the panels requires following the timing diagram in > > each panel's datasheet which specifies delays between certain > > actions. This means that, while we can be quite dynamic about handling > > things we can't just totally skip out on describing the panel like we > > could do if it was connected to an external-facing DP port. > > Is this a 'standard' eDP connector? AFAICT, there does seem to be > such a thing. To answer this one: there's not any "standard" physical plug as far as I can tell. There's a connector on the board side for the LCD that has a whole hodgepodge of signals on it. Maybe USB for a camera. Some power signals. Maybe a PWM for a backlight. Maybe some DMIC signals. eDP signals which might be anywhere from 1 to 4 lanes. HPD (which is really a "panel ready" signal for eDP). The size / style of connector and the exact set of signals (and their ordering) is board specific. You then get a board-specific cable that splits things out. Some might go to a camera/MIC sub board. Some go to the panel and hook onto a panel-specific connector which has pin count and orderings defined by that panel. :-P > I've said in the past I'd be okay with a edp-connector > node. If that needs just the "HPD absent delay" property, I think that > would be okay. It's just a never ending stream of new properties with > each new panel that I don't want to see. Thinking about this we'd need at least one other property right now which is an enable delay. Specifically at least one panel I've supported recently lied about HPD for a short period after bootup. Specifically see commit 667d73d72f31 ("drm: panel: simple: Delay HPD checking on boe_nv133fhm_n61 for 15 ms"). ...and, of course, the existing power supply / enable signals that "simple-panel" already has. Also: if we weren't going to add the other delay properties in the device tree, we'd have to add the code right away that used the EDID to set other delays. That wouldn't be the end of the world, but it would be code to write. One last thought to add: I've looked at ~10 panels specs recently. Though they are all a little different from each other, I will say that almost every one of them seems to have the exact same timing diagram in it just with different numbers filled in. To me that backs up the idea that you can/should do the power sequence with a fairly standard (parameterized) driver. I can't link the datasheets I have but searching for "edp panel datasheet" finds me this random datasheet: https://www.data-modul.com/sites/default/files/products/NV156QUM-N72_specification_12039472.pdf See "8.0 POWER SEQUENCE" in that document. All the panels have a nearly identical diagram with different numbers filled in. You can kinda tell it was copied from some other panel since some numbers (like T4) aren't even defined. -Doug ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-03-29 16:25 ` Doug Anderson 0 siblings, 0 replies; 40+ messages in thread From: Doug Anderson @ 2021-03-29 16:25 UTC (permalink / raw) To: Rob Herring Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Thierry Reding, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, LKML Hi, On Thu, Mar 25, 2021 at 5:09 PM Rob Herring <robh@kernel.org> wrote: > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > The sc7180-trogdor-pompom board might be attached to any number of a > > pile of eDP panels. At the moment I'm told that the list might include: > > - KD KD116N21-30NV-A010 > > - KD KD116N09-30NH-A016 > > - Starry 2081116HHD028001-51D > > - Sharp LQ116M1JW10 > > > > It should be noted that while the EDID programmed in the first 3 > > panels indicates that they should run with exactly the same timing (to > > keep things simple), the 4th panel not only needs different timing but > > has a different resolution. > > > > As is true in general with eDP panels, we can figure out which panel > > we have and all the info needed to drive its pixel clock by reading > > the EDID. However, we can do this only after we've powered the panel > > on. Powering on the panels requires following the timing diagram in > > each panel's datasheet which specifies delays between certain > > actions. This means that, while we can be quite dynamic about handling > > things we can't just totally skip out on describing the panel like we > > could do if it was connected to an external-facing DP port. > > Is this a 'standard' eDP connector? AFAICT, there does seem to be > such a thing. To answer this one: there's not any "standard" physical plug as far as I can tell. There's a connector on the board side for the LCD that has a whole hodgepodge of signals on it. Maybe USB for a camera. Some power signals. Maybe a PWM for a backlight. Maybe some DMIC signals. eDP signals which might be anywhere from 1 to 4 lanes. HPD (which is really a "panel ready" signal for eDP). The size / style of connector and the exact set of signals (and their ordering) is board specific. You then get a board-specific cable that splits things out. Some might go to a camera/MIC sub board. Some go to the panel and hook onto a panel-specific connector which has pin count and orderings defined by that panel. :-P > I've said in the past I'd be okay with a edp-connector > node. If that needs just the "HPD absent delay" property, I think that > would be okay. It's just a never ending stream of new properties with > each new panel that I don't want to see. Thinking about this we'd need at least one other property right now which is an enable delay. Specifically at least one panel I've supported recently lied about HPD for a short period after bootup. Specifically see commit 667d73d72f31 ("drm: panel: simple: Delay HPD checking on boe_nv133fhm_n61 for 15 ms"). ...and, of course, the existing power supply / enable signals that "simple-panel" already has. Also: if we weren't going to add the other delay properties in the device tree, we'd have to add the code right away that used the EDID to set other delays. That wouldn't be the end of the world, but it would be code to write. One last thought to add: I've looked at ~10 panels specs recently. Though they are all a little different from each other, I will say that almost every one of them seems to have the exact same timing diagram in it just with different numbers filled in. To me that backs up the idea that you can/should do the power sequence with a fairly standard (parameterized) driver. I can't link the datasheets I have but searching for "edp panel datasheet" finds me this random datasheet: https://www.data-modul.com/sites/default/files/products/NV156QUM-N72_specification_12039472.pdf See "8.0 POWER SEQUENCE" in that document. All the panels have a nearly identical diagram with different numbers filled in. You can kinda tell it was copied from some other panel since some numbers (like T4) aren't even defined. -Doug _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-03-29 16:25 ` Doug Anderson @ 2021-04-22 22:08 ` Doug Anderson -1 siblings, 0 replies; 40+ messages in thread From: Doug Anderson @ 2021-04-22 22:08 UTC (permalink / raw) To: Rob Herring Cc: Bjorn Andersson, Thierry Reding, Sam Ravnborg, Rob Clark, Daniel Vetter, dri-devel, David Airlie, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Stephen Boyd, linux-arm-msm, Steev Klimaszewski, Enric Balletbo Serra, Andy Gross, Linus Walleij, Nicolas Boichat, LKML, Laurent Pinchart Hi, On Mon, Mar 29, 2021 at 9:25 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Mar 25, 2021 at 5:09 PM Rob Herring <robh@kernel.org> wrote: > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > pile of eDP panels. At the moment I'm told that the list might include: > > > - KD KD116N21-30NV-A010 > > > - KD KD116N09-30NH-A016 > > > - Starry 2081116HHD028001-51D > > > - Sharp LQ116M1JW10 > > > > > > It should be noted that while the EDID programmed in the first 3 > > > panels indicates that they should run with exactly the same timing (to > > > keep things simple), the 4th panel not only needs different timing but > > > has a different resolution. > > > > > > As is true in general with eDP panels, we can figure out which panel > > > we have and all the info needed to drive its pixel clock by reading > > > the EDID. However, we can do this only after we've powered the panel > > > on. Powering on the panels requires following the timing diagram in > > > each panel's datasheet which specifies delays between certain > > > actions. This means that, while we can be quite dynamic about handling > > > things we can't just totally skip out on describing the panel like we > > > could do if it was connected to an external-facing DP port. > > > > Is this a 'standard' eDP connector? AFAICT, there does seem to be > > such a thing. > > To answer this one: there's not any "standard" physical plug as far as > I can tell. There's a connector on the board side for the LCD that has > a whole hodgepodge of signals on it. Maybe USB for a camera. Some > power signals. Maybe a PWM for a backlight. Maybe some DMIC signals. > eDP signals which might be anywhere from 1 to 4 lanes. HPD (which is > really a "panel ready" signal for eDP). The size / style of connector > and the exact set of signals (and their ordering) is board specific. > You then get a board-specific cable that splits things out. Some might > go to a camera/MIC sub board. Some go to the panel and hook onto a > panel-specific connector which has pin count and orderings defined by > that panel. :-P > > > > I've said in the past I'd be okay with a edp-connector > > node. If that needs just the "HPD absent delay" property, I think that > > would be okay. It's just a never ending stream of new properties with > > each new panel that I don't want to see. > > Thinking about this we'd need at least one other property right now > which is an enable delay. Specifically at least one panel I've > supported recently lied about HPD for a short period after bootup. > Specifically see commit 667d73d72f31 ("drm: panel: simple: Delay HPD > checking on boe_nv133fhm_n61 for 15 ms"). ...and, of course, the > existing power supply / enable signals that "simple-panel" already > has. > > Also: if we weren't going to add the other delay properties in the > device tree, we'd have to add the code right away that used the EDID > to set other delays. That wouldn't be the end of the world, but it > would be code to write. > > > One last thought to add: I've looked at ~10 panels specs recently. > Though they are all a little different from each other, I will say > that almost every one of them seems to have the exact same timing > diagram in it just with different numbers filled in. To me that backs > up the idea that you can/should do the power sequence with a fairly > standard (parameterized) driver. I can't link the datasheets I have > but searching for "edp panel datasheet" finds me this random > datasheet: > > https://www.data-modul.com/sites/default/files/products/NV156QUM-N72_specification_12039472.pdf > > See "8.0 POWER SEQUENCE" in that document. All the panels have a > nearly identical diagram with different numbers filled in. You can > kinda tell it was copied from some other panel since some numbers > (like T4) aren't even defined. So this thread has been quiet for a while, but the problem still exists. Here's my current plan, but please yell if you disagree: 1. See about adding a generic "eDP connector" node. Having stewed on this for a while I think I'm convinced that even though there's not really a single standard physical connector that is used everywhere that there are at least a set of signals that can be collectively thought about as the "eDP signals". Certainly I have a set of very different panels from very different manufacturers that I can "interchange" and they work fine assuming I have the right cable "adapting" them from the connector on my board to the connector on the panel. While different panels have different timings that they care are enforced, there is a way to express it in a relatively common way as evidenced by the fact that all panel datasheet timing diagrams look similar and the fact that panel-simple handles so many different panels (yes, we periodically add more timing constraints to handle there but mostly that's because the code wasn't able to handle every constraint that could be expressed in those standard-looking timing diagrams in the datasheets). 2. The "eDP connector" node will have all the same properties as today's "panel-simple.yaml" with the addition of: enable-delay hpd-absent-delay The idea is that you power on the panel, hardcode an enable-delay (to handle early HPD glitches), and then wait for HPD (or wait hpd-absent-delay if HPD isn't provided). Note that "ddc-i2c-bus" will be a required node instead of optional. 3. Once we power the panel on then we will query the EDID and set the rest of the panel timings / modes based on the model specified in the EDID. Potentially it could update the "enable-delay" and "hpd-absent-delay" at this point too. We can thumb wrestle about whether the code to support this lives in "panel-simple.c" or not. -Doug ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-04-22 22:08 ` Doug Anderson 0 siblings, 0 replies; 40+ messages in thread From: Doug Anderson @ 2021-04-22 22:08 UTC (permalink / raw) To: Rob Herring Cc: Rob Clark, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Nicolas Boichat, David Airlie, linux-arm-msm, Andy Gross, dri-devel, Bjorn Andersson, Thierry Reding, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, LKML, Laurent Pinchart Hi, On Mon, Mar 29, 2021 at 9:25 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Mar 25, 2021 at 5:09 PM Rob Herring <robh@kernel.org> wrote: > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > pile of eDP panels. At the moment I'm told that the list might include: > > > - KD KD116N21-30NV-A010 > > > - KD KD116N09-30NH-A016 > > > - Starry 2081116HHD028001-51D > > > - Sharp LQ116M1JW10 > > > > > > It should be noted that while the EDID programmed in the first 3 > > > panels indicates that they should run with exactly the same timing (to > > > keep things simple), the 4th panel not only needs different timing but > > > has a different resolution. > > > > > > As is true in general with eDP panels, we can figure out which panel > > > we have and all the info needed to drive its pixel clock by reading > > > the EDID. However, we can do this only after we've powered the panel > > > on. Powering on the panels requires following the timing diagram in > > > each panel's datasheet which specifies delays between certain > > > actions. This means that, while we can be quite dynamic about handling > > > things we can't just totally skip out on describing the panel like we > > > could do if it was connected to an external-facing DP port. > > > > Is this a 'standard' eDP connector? AFAICT, there does seem to be > > such a thing. > > To answer this one: there's not any "standard" physical plug as far as > I can tell. There's a connector on the board side for the LCD that has > a whole hodgepodge of signals on it. Maybe USB for a camera. Some > power signals. Maybe a PWM for a backlight. Maybe some DMIC signals. > eDP signals which might be anywhere from 1 to 4 lanes. HPD (which is > really a "panel ready" signal for eDP). The size / style of connector > and the exact set of signals (and their ordering) is board specific. > You then get a board-specific cable that splits things out. Some might > go to a camera/MIC sub board. Some go to the panel and hook onto a > panel-specific connector which has pin count and orderings defined by > that panel. :-P > > > > I've said in the past I'd be okay with a edp-connector > > node. If that needs just the "HPD absent delay" property, I think that > > would be okay. It's just a never ending stream of new properties with > > each new panel that I don't want to see. > > Thinking about this we'd need at least one other property right now > which is an enable delay. Specifically at least one panel I've > supported recently lied about HPD for a short period after bootup. > Specifically see commit 667d73d72f31 ("drm: panel: simple: Delay HPD > checking on boe_nv133fhm_n61 for 15 ms"). ...and, of course, the > existing power supply / enable signals that "simple-panel" already > has. > > Also: if we weren't going to add the other delay properties in the > device tree, we'd have to add the code right away that used the EDID > to set other delays. That wouldn't be the end of the world, but it > would be code to write. > > > One last thought to add: I've looked at ~10 panels specs recently. > Though they are all a little different from each other, I will say > that almost every one of them seems to have the exact same timing > diagram in it just with different numbers filled in. To me that backs > up the idea that you can/should do the power sequence with a fairly > standard (parameterized) driver. I can't link the datasheets I have > but searching for "edp panel datasheet" finds me this random > datasheet: > > https://www.data-modul.com/sites/default/files/products/NV156QUM-N72_specification_12039472.pdf > > See "8.0 POWER SEQUENCE" in that document. All the panels have a > nearly identical diagram with different numbers filled in. You can > kinda tell it was copied from some other panel since some numbers > (like T4) aren't even defined. So this thread has been quiet for a while, but the problem still exists. Here's my current plan, but please yell if you disagree: 1. See about adding a generic "eDP connector" node. Having stewed on this for a while I think I'm convinced that even though there's not really a single standard physical connector that is used everywhere that there are at least a set of signals that can be collectively thought about as the "eDP signals". Certainly I have a set of very different panels from very different manufacturers that I can "interchange" and they work fine assuming I have the right cable "adapting" them from the connector on my board to the connector on the panel. While different panels have different timings that they care are enforced, there is a way to express it in a relatively common way as evidenced by the fact that all panel datasheet timing diagrams look similar and the fact that panel-simple handles so many different panels (yes, we periodically add more timing constraints to handle there but mostly that's because the code wasn't able to handle every constraint that could be expressed in those standard-looking timing diagrams in the datasheets). 2. The "eDP connector" node will have all the same properties as today's "panel-simple.yaml" with the addition of: enable-delay hpd-absent-delay The idea is that you power on the panel, hardcode an enable-delay (to handle early HPD glitches), and then wait for HPD (or wait hpd-absent-delay if HPD isn't provided). Note that "ddc-i2c-bus" will be a required node instead of optional. 3. Once we power the panel on then we will query the EDID and set the rest of the panel timings / modes based on the model specified in the EDID. Potentially it could update the "enable-delay" and "hpd-absent-delay" at this point too. We can thumb wrestle about whether the code to support this lives in "panel-simple.c" or not. -Doug _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom 2021-04-22 22:08 ` Doug Anderson @ 2021-04-23 11:14 ` Thierry Reding -1 siblings, 0 replies; 40+ messages in thread From: Thierry Reding @ 2021-04-23 11:14 UTC (permalink / raw) To: Doug Anderson Cc: Rob Herring, Bjorn Andersson, Sam Ravnborg, Rob Clark, Daniel Vetter, dri-devel, David Airlie, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Stephen Boyd, linux-arm-msm, Steev Klimaszewski, Enric Balletbo Serra, Andy Gross, Linus Walleij, Nicolas Boichat, LKML, Laurent Pinchart [-- Attachment #1: Type: text/plain, Size: 6837 bytes --] On Thu, Apr 22, 2021 at 03:08:48PM -0700, Doug Anderson wrote: > Hi, > > On Mon, Mar 29, 2021 at 9:25 AM Doug Anderson <dianders@chromium.org> wrote: > > > > Hi, > > > > On Thu, Mar 25, 2021 at 5:09 PM Rob Herring <robh@kernel.org> wrote: > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > - KD KD116N21-30NV-A010 > > > > - KD KD116N09-30NH-A016 > > > > - Starry 2081116HHD028001-51D > > > > - Sharp LQ116M1JW10 > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > panels indicates that they should run with exactly the same timing (to > > > > keep things simple), the 4th panel not only needs different timing but > > > > has a different resolution. > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > we have and all the info needed to drive its pixel clock by reading > > > > the EDID. However, we can do this only after we've powered the panel > > > > on. Powering on the panels requires following the timing diagram in > > > > each panel's datasheet which specifies delays between certain > > > > actions. This means that, while we can be quite dynamic about handling > > > > things we can't just totally skip out on describing the panel like we > > > > could do if it was connected to an external-facing DP port. > > > > > > Is this a 'standard' eDP connector? AFAICT, there does seem to be > > > such a thing. > > > > To answer this one: there's not any "standard" physical plug as far as > > I can tell. There's a connector on the board side for the LCD that has > > a whole hodgepodge of signals on it. Maybe USB for a camera. Some > > power signals. Maybe a PWM for a backlight. Maybe some DMIC signals. > > eDP signals which might be anywhere from 1 to 4 lanes. HPD (which is > > really a "panel ready" signal for eDP). The size / style of connector > > and the exact set of signals (and their ordering) is board specific. > > You then get a board-specific cable that splits things out. Some might > > go to a camera/MIC sub board. Some go to the panel and hook onto a > > panel-specific connector which has pin count and orderings defined by > > that panel. :-P > > > > > > > I've said in the past I'd be okay with a edp-connector > > > node. If that needs just the "HPD absent delay" property, I think that > > > would be okay. It's just a never ending stream of new properties with > > > each new panel that I don't want to see. > > > > Thinking about this we'd need at least one other property right now > > which is an enable delay. Specifically at least one panel I've > > supported recently lied about HPD for a short period after bootup. > > Specifically see commit 667d73d72f31 ("drm: panel: simple: Delay HPD > > checking on boe_nv133fhm_n61 for 15 ms"). ...and, of course, the > > existing power supply / enable signals that "simple-panel" already > > has. > > > > Also: if we weren't going to add the other delay properties in the > > device tree, we'd have to add the code right away that used the EDID > > to set other delays. That wouldn't be the end of the world, but it > > would be code to write. > > > > > > One last thought to add: I've looked at ~10 panels specs recently. > > Though they are all a little different from each other, I will say > > that almost every one of them seems to have the exact same timing > > diagram in it just with different numbers filled in. To me that backs > > up the idea that you can/should do the power sequence with a fairly > > standard (parameterized) driver. I can't link the datasheets I have > > but searching for "edp panel datasheet" finds me this random > > datasheet: > > > > https://www.data-modul.com/sites/default/files/products/NV156QUM-N72_specification_12039472.pdf > > > > See "8.0 POWER SEQUENCE" in that document. All the panels have a > > nearly identical diagram with different numbers filled in. You can > > kinda tell it was copied from some other panel since some numbers > > (like T4) aren't even defined. > > So this thread has been quiet for a while, but the problem still exists. > > Here's my current plan, but please yell if you disagree: > > 1. See about adding a generic "eDP connector" node. Having stewed on > this for a while I think I'm convinced that even though there's not > really a single standard physical connector that is used everywhere > that there are at least a set of signals that can be collectively > thought about as the "eDP signals". Certainly I have a set of very > different panels from very different manufacturers that I can > "interchange" and they work fine assuming I have the right cable > "adapting" them from the connector on my board to the connector on the > panel. While different panels have different timings that they care > are enforced, there is a way to express it in a relatively common way > as evidenced by the fact that all panel datasheet timing diagrams look > similar and the fact that panel-simple handles so many different > panels (yes, we periodically add more timing constraints to handle > there but mostly that's because the code wasn't able to handle every > constraint that could be expressed in those standard-looking timing > diagrams in the datasheets). > > > 2. The "eDP connector" node will have all the same properties as > today's "panel-simple.yaml" with the addition of: > > enable-delay > hpd-absent-delay > > The idea is that you power on the panel, hardcode an enable-delay (to > handle early HPD glitches), and then wait for HPD (or wait > hpd-absent-delay if HPD isn't provided). > > Note that "ddc-i2c-bus" will be a required node instead of optional. > > > 3. Once we power the panel on then we will query the EDID and set the > rest of the panel timings / modes based on the model specified in the > EDID. Potentially it could update the "enable-delay" and > "hpd-absent-delay" at this point too. I think that sounds good. If ddc-i2c-bus is required, this basically implies that EDID needs to be available for these panels, too. If that's the case we can identify the panel based on information from the EDID. That would make panels "discoverable", so that we can describe them with a more generic compatible string that basically describes the interface needed to get at the discoverable information, much like we would do for a bus like PCI. I don't know if the manufacturer ID and product code are enough to uniquely identify every panel, but maybe something like the DisplayID extension can be used to gather more identification data. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom @ 2021-04-23 11:14 ` Thierry Reding 0 siblings, 0 replies; 40+ messages in thread From: Thierry Reding @ 2021-04-23 11:14 UTC (permalink / raw) To: Doug Anderson Cc: Rob Clark, Nicolas Boichat, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, David Airlie, linux-arm-msm, LKML, dri-devel, Bjorn Andersson, Andy Gross, Steev Klimaszewski, Stephen Boyd, Sam Ravnborg, Laurent Pinchart [-- Attachment #1.1: Type: text/plain, Size: 6837 bytes --] On Thu, Apr 22, 2021 at 03:08:48PM -0700, Doug Anderson wrote: > Hi, > > On Mon, Mar 29, 2021 at 9:25 AM Doug Anderson <dianders@chromium.org> wrote: > > > > Hi, > > > > On Thu, Mar 25, 2021 at 5:09 PM Rob Herring <robh@kernel.org> wrote: > > > > > > On Tue, Mar 16, 2021 at 02:08:19PM -0700, Douglas Anderson wrote: > > > > The sc7180-trogdor-pompom board might be attached to any number of a > > > > pile of eDP panels. At the moment I'm told that the list might include: > > > > - KD KD116N21-30NV-A010 > > > > - KD KD116N09-30NH-A016 > > > > - Starry 2081116HHD028001-51D > > > > - Sharp LQ116M1JW10 > > > > > > > > It should be noted that while the EDID programmed in the first 3 > > > > panels indicates that they should run with exactly the same timing (to > > > > keep things simple), the 4th panel not only needs different timing but > > > > has a different resolution. > > > > > > > > As is true in general with eDP panels, we can figure out which panel > > > > we have and all the info needed to drive its pixel clock by reading > > > > the EDID. However, we can do this only after we've powered the panel > > > > on. Powering on the panels requires following the timing diagram in > > > > each panel's datasheet which specifies delays between certain > > > > actions. This means that, while we can be quite dynamic about handling > > > > things we can't just totally skip out on describing the panel like we > > > > could do if it was connected to an external-facing DP port. > > > > > > Is this a 'standard' eDP connector? AFAICT, there does seem to be > > > such a thing. > > > > To answer this one: there's not any "standard" physical plug as far as > > I can tell. There's a connector on the board side for the LCD that has > > a whole hodgepodge of signals on it. Maybe USB for a camera. Some > > power signals. Maybe a PWM for a backlight. Maybe some DMIC signals. > > eDP signals which might be anywhere from 1 to 4 lanes. HPD (which is > > really a "panel ready" signal for eDP). The size / style of connector > > and the exact set of signals (and their ordering) is board specific. > > You then get a board-specific cable that splits things out. Some might > > go to a camera/MIC sub board. Some go to the panel and hook onto a > > panel-specific connector which has pin count and orderings defined by > > that panel. :-P > > > > > > > I've said in the past I'd be okay with a edp-connector > > > node. If that needs just the "HPD absent delay" property, I think that > > > would be okay. It's just a never ending stream of new properties with > > > each new panel that I don't want to see. > > > > Thinking about this we'd need at least one other property right now > > which is an enable delay. Specifically at least one panel I've > > supported recently lied about HPD for a short period after bootup. > > Specifically see commit 667d73d72f31 ("drm: panel: simple: Delay HPD > > checking on boe_nv133fhm_n61 for 15 ms"). ...and, of course, the > > existing power supply / enable signals that "simple-panel" already > > has. > > > > Also: if we weren't going to add the other delay properties in the > > device tree, we'd have to add the code right away that used the EDID > > to set other delays. That wouldn't be the end of the world, but it > > would be code to write. > > > > > > One last thought to add: I've looked at ~10 panels specs recently. > > Though they are all a little different from each other, I will say > > that almost every one of them seems to have the exact same timing > > diagram in it just with different numbers filled in. To me that backs > > up the idea that you can/should do the power sequence with a fairly > > standard (parameterized) driver. I can't link the datasheets I have > > but searching for "edp panel datasheet" finds me this random > > datasheet: > > > > https://www.data-modul.com/sites/default/files/products/NV156QUM-N72_specification_12039472.pdf > > > > See "8.0 POWER SEQUENCE" in that document. All the panels have a > > nearly identical diagram with different numbers filled in. You can > > kinda tell it was copied from some other panel since some numbers > > (like T4) aren't even defined. > > So this thread has been quiet for a while, but the problem still exists. > > Here's my current plan, but please yell if you disagree: > > 1. See about adding a generic "eDP connector" node. Having stewed on > this for a while I think I'm convinced that even though there's not > really a single standard physical connector that is used everywhere > that there are at least a set of signals that can be collectively > thought about as the "eDP signals". Certainly I have a set of very > different panels from very different manufacturers that I can > "interchange" and they work fine assuming I have the right cable > "adapting" them from the connector on my board to the connector on the > panel. While different panels have different timings that they care > are enforced, there is a way to express it in a relatively common way > as evidenced by the fact that all panel datasheet timing diagrams look > similar and the fact that panel-simple handles so many different > panels (yes, we periodically add more timing constraints to handle > there but mostly that's because the code wasn't able to handle every > constraint that could be expressed in those standard-looking timing > diagrams in the datasheets). > > > 2. The "eDP connector" node will have all the same properties as > today's "panel-simple.yaml" with the addition of: > > enable-delay > hpd-absent-delay > > The idea is that you power on the panel, hardcode an enable-delay (to > handle early HPD glitches), and then wait for HPD (or wait > hpd-absent-delay if HPD isn't provided). > > Note that "ddc-i2c-bus" will be a required node instead of optional. > > > 3. Once we power the panel on then we will query the EDID and set the > rest of the panel timings / modes based on the model specified in the > EDID. Potentially it could update the "enable-delay" and > "hpd-absent-delay" at this point too. I think that sounds good. If ddc-i2c-bus is required, this basically implies that EDID needs to be available for these panels, too. If that's the case we can identify the panel based on information from the EDID. That would make panels "discoverable", so that we can describe them with a more generic compatible string that basically describes the interface needed to get at the discoverable information, much like we would do for a bus like PCI. I don't know if the manufacturer ID and product code are enough to uniquely identify every panel, but maybe something like the DisplayID extension can be used to gather more identification data. Thierry [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2021-04-23 11:13 UTC | newest] Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-16 21:08 [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom Douglas Anderson 2021-03-16 21:08 ` Douglas Anderson 2021-03-16 21:08 ` [RFC PATCH 2/3] drm/panel: panel-simple: " Douglas Anderson 2021-03-16 21:08 ` Douglas Anderson 2021-03-16 21:08 ` [RFC PATCH 3/3] arm64: dts: qcom: Don't specify exactly what panel is on pompom Douglas Anderson 2021-03-16 21:08 ` Douglas Anderson 2021-03-17 23:27 ` [RFC PATCH 1/3] dt-bindings: display: simple: Add the panel on sc7180-trogdor-pompom Matthias Kaehlcke 2021-03-17 23:27 ` Matthias Kaehlcke 2021-03-18 1:53 ` Rob Clark 2021-03-18 1:53 ` Rob Clark 2021-03-26 12:39 ` Thierry Reding 2021-03-26 12:39 ` Thierry Reding 2021-03-26 15:18 ` Rob Clark 2021-03-26 15:18 ` Rob Clark 2021-03-26 15:24 ` Rob Clark 2021-03-26 15:24 ` Rob Clark 2021-03-26 17:00 ` Bjorn Andersson 2021-03-26 17:00 ` Bjorn Andersson 2021-03-26 19:48 ` Rob Herring 2021-03-26 19:48 ` Rob Herring 2021-03-26 22:16 ` Rob Clark 2021-03-26 22:16 ` Rob Clark 2021-03-26 23:48 ` Rob Herring 2021-03-26 23:48 ` Rob Herring 2021-03-27 0:33 ` Rob Clark 2021-03-27 0:33 ` Rob Clark 2021-03-27 0:49 ` Rob Clark 2021-03-27 0:49 ` Rob Clark 2021-03-29 15:51 ` Doug Anderson 2021-03-29 15:51 ` Doug Anderson 2021-03-29 16:07 ` Doug Anderson 2021-03-29 16:07 ` Doug Anderson 2021-03-26 0:09 ` Rob Herring 2021-03-26 0:09 ` Rob Herring 2021-03-29 16:25 ` Doug Anderson 2021-03-29 16:25 ` Doug Anderson 2021-04-22 22:08 ` Doug Anderson 2021-04-22 22:08 ` Doug Anderson 2021-04-23 11:14 ` Thierry Reding 2021-04-23 11:14 ` Thierry Reding
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.