| Message ID | 20230314110043.2139111-2-treapking@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/bridge: ps8640: Skip redundant bridge enable | expand |
On Tue, Mar 14, 2023 at 12:01 PM Pin-yen Lin <treapking@chromium.org> wrote: > > When there are multiple EDID reads, the bridge will be repeatedly > enabled and disabled. Add a cache for EDID to speed this up. > > Signed-off-by: Pin-yen Lin <treapking@chromium.org> > --- > > drivers/gpu/drm/bridge/parade-ps8640.c | 61 ++++++++++++++++---------- > 1 file changed, 37 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c > index 08de501c436e..4d594be8b89c 100644 > --- a/drivers/gpu/drm/bridge/parade-ps8640.c > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c > @@ -105,6 +105,7 @@ struct ps8640 { > struct gpio_desc *gpio_reset; > struct gpio_desc *gpio_powerdown; > struct device_link *link; > + struct edid *edid; > bool pre_enabled; > bool need_post_hpd_delay; > }; > @@ -543,34 +544,37 @@ static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge, > { > struct ps8640 *ps_bridge = bridge_to_ps8640(bridge); > bool poweroff = !ps_bridge->pre_enabled; > - struct edid *edid; > > - /* > - * When we end calling get_edid() triggered by an ioctl, i.e > - * > - * drm_mode_getconnector (ioctl) > - * -> drm_helper_probe_single_connector_modes > - * -> drm_bridge_connector_get_modes > - * -> ps8640_bridge_get_edid > - * > - * We need to make sure that what we need is enabled before reading > - * EDID, for this chip, we need to do a full poweron, otherwise it will > - * fail. > - */ > - if (poweroff) > - drm_atomic_bridge_chain_pre_enable(bridge, connector->state->state); > + if (!ps_bridge->edid) { > + /* > + * When we end calling get_edid() triggered by an ioctl, i.e > + * > + * drm_mode_getconnector (ioctl) > + * -> drm_helper_probe_single_connector_modes > + * -> drm_bridge_connector_get_modes > + * -> ps8640_bridge_get_edid > + * > + * We need to make sure that what we need is enabled before > + * reading EDID, for this chip, we need to do a full poweron, > + * otherwise it will fail. > + */ > + if (poweroff) > + drm_atomic_bridge_chain_pre_enable(bridge, > + connector->state->state); > > - edid = drm_get_edid(connector, > - ps_bridge->page[PAGE0_DP_CNTL]->adapter); > + ps_bridge->edid = drm_get_edid(connector, > + ps_bridge->page[PAGE0_DP_CNTL]->adapter); > > - /* > - * If we call the get_edid() function without having enabled the chip > - * before, return the chip to its original power state. > - */ > - if (poweroff) > - drm_atomic_bridge_chain_post_disable(bridge, connector->state->state); > + /* > + * If we call the get_edid() function without having enabled the > + * chip before, return the chip to its original power state. > + */ > + if (poweroff) > + drm_atomic_bridge_chain_post_disable(bridge, > + connector->state->state); > + } > > - return edid; > + return drm_edid_duplicate(ps_bridge->edid); > } > > static void ps8640_runtime_disable(void *data) > @@ -767,6 +771,14 @@ static int ps8640_probe(struct i2c_client *client) > return ret; > } > > +static void ps8640_remove(struct i2c_client *client) > +{ > + struct ps8640 *ps_bridge = i2c_get_clientdata(client); > + > + kfree(ps_bridge->edid); > + ps_bridge->edid = NULL; > +} > + > static const struct of_device_id ps8640_match[] = { > { .compatible = "parade,ps8640" }, > { } > @@ -775,6 +787,7 @@ MODULE_DEVICE_TABLE(of, ps8640_match); > > static struct i2c_driver ps8640_driver = { > .probe_new = ps8640_probe, > + .remove = ps8640_remove, > .driver = { > .name = "ps8640", > .of_match_table = ps8640_match, > -- > 2.40.0.rc1.284.g88254d51c5-goog > Reviewed-by: Robert Foss <rfoss@kernel.org>
Hi, On Tue, Mar 14, 2023 at 4:00 AM Pin-yen Lin <treapking@chromium.org> wrote: > > When there are multiple EDID reads, the bridge will be repeatedly > enabled and disabled. Add a cache for EDID to speed this up. > > Signed-off-by: Pin-yen Lin <treapking@chromium.org> > --- > > drivers/gpu/drm/bridge/parade-ps8640.c | 61 ++++++++++++++++---------- > 1 file changed, 37 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c > index 08de501c436e..4d594be8b89c 100644 > --- a/drivers/gpu/drm/bridge/parade-ps8640.c > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c > @@ -105,6 +105,7 @@ struct ps8640 { > struct gpio_desc *gpio_reset; > struct gpio_desc *gpio_powerdown; > struct device_link *link; > + struct edid *edid; > bool pre_enabled; > bool need_post_hpd_delay; > }; > @@ -543,34 +544,37 @@ static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge, > { > struct ps8640 *ps_bridge = bridge_to_ps8640(bridge); > bool poweroff = !ps_bridge->pre_enabled; > - struct edid *edid; > > - /* > - * When we end calling get_edid() triggered by an ioctl, i.e > - * > - * drm_mode_getconnector (ioctl) > - * -> drm_helper_probe_single_connector_modes > - * -> drm_bridge_connector_get_modes > - * -> ps8640_bridge_get_edid > - * > - * We need to make sure that what we need is enabled before reading > - * EDID, for this chip, we need to do a full poweron, otherwise it will > - * fail. > - */ > - if (poweroff) > - drm_atomic_bridge_chain_pre_enable(bridge, connector->state->state); > + if (!ps_bridge->edid) { > + /* > + * When we end calling get_edid() triggered by an ioctl, i.e > + * > + * drm_mode_getconnector (ioctl) > + * -> drm_helper_probe_single_connector_modes > + * -> drm_bridge_connector_get_modes > + * -> ps8640_bridge_get_edid > + * > + * We need to make sure that what we need is enabled before > + * reading EDID, for this chip, we need to do a full poweron, > + * otherwise it will fail. > + */ > + if (poweroff) > + drm_atomic_bridge_chain_pre_enable(bridge, > + connector->state->state); > > - edid = drm_get_edid(connector, > - ps_bridge->page[PAGE0_DP_CNTL]->adapter); > + ps_bridge->edid = drm_get_edid(connector, > + ps_bridge->page[PAGE0_DP_CNTL]->adapter); > > - /* > - * If we call the get_edid() function without having enabled the chip > - * before, return the chip to its original power state. > - */ > - if (poweroff) > - drm_atomic_bridge_chain_post_disable(bridge, connector->state->state); > + /* > + * If we call the get_edid() function without having enabled the > + * chip before, return the chip to its original power state. > + */ > + if (poweroff) > + drm_atomic_bridge_chain_post_disable(bridge, > + connector->state->state); > + } > > - return edid; > + return drm_edid_duplicate(ps_bridge->edid); > } > > static void ps8640_runtime_disable(void *data) > @@ -767,6 +771,14 @@ static int ps8640_probe(struct i2c_client *client) > return ret; > } > > +static void ps8640_remove(struct i2c_client *client) > +{ > + struct ps8640 *ps_bridge = i2c_get_clientdata(client); > + > + kfree(ps_bridge->edid); > + ps_bridge->edid = NULL; nit: no need to clear this to NULL since the driver is exiting. Other than the small nit: Reviewed-by: Douglas Anderson <dianders@chromium.org>
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c index 08de501c436e..4d594be8b89c 100644 --- a/drivers/gpu/drm/bridge/parade-ps8640.c +++ b/drivers/gpu/drm/bridge/parade-ps8640.c @@ -105,6 +105,7 @@ struct ps8640 { struct gpio_desc *gpio_reset; struct gpio_desc *gpio_powerdown; struct device_link *link; + struct edid *edid; bool pre_enabled; bool need_post_hpd_delay; }; @@ -543,34 +544,37 @@ static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge, { struct ps8640 *ps_bridge = bridge_to_ps8640(bridge); bool poweroff = !ps_bridge->pre_enabled; - struct edid *edid; - /* - * When we end calling get_edid() triggered by an ioctl, i.e - * - * drm_mode_getconnector (ioctl) - * -> drm_helper_probe_single_connector_modes - * -> drm_bridge_connector_get_modes - * -> ps8640_bridge_get_edid - * - * We need to make sure that what we need is enabled before reading - * EDID, for this chip, we need to do a full poweron, otherwise it will - * fail. - */ - if (poweroff) - drm_atomic_bridge_chain_pre_enable(bridge, connector->state->state); + if (!ps_bridge->edid) { + /* + * When we end calling get_edid() triggered by an ioctl, i.e + * + * drm_mode_getconnector (ioctl) + * -> drm_helper_probe_single_connector_modes + * -> drm_bridge_connector_get_modes + * -> ps8640_bridge_get_edid + * + * We need to make sure that what we need is enabled before + * reading EDID, for this chip, we need to do a full poweron, + * otherwise it will fail. + */ + if (poweroff) + drm_atomic_bridge_chain_pre_enable(bridge, + connector->state->state); - edid = drm_get_edid(connector, - ps_bridge->page[PAGE0_DP_CNTL]->adapter); + ps_bridge->edid = drm_get_edid(connector, + ps_bridge->page[PAGE0_DP_CNTL]->adapter); - /* - * If we call the get_edid() function without having enabled the chip - * before, return the chip to its original power state. - */ - if (poweroff) - drm_atomic_bridge_chain_post_disable(bridge, connector->state->state); + /* + * If we call the get_edid() function without having enabled the + * chip before, return the chip to its original power state. + */ + if (poweroff) + drm_atomic_bridge_chain_post_disable(bridge, + connector->state->state); + } - return edid; + return drm_edid_duplicate(ps_bridge->edid); } static void ps8640_runtime_disable(void *data) @@ -767,6 +771,14 @@ static int ps8640_probe(struct i2c_client *client) return ret; } +static void ps8640_remove(struct i2c_client *client) +{ + struct ps8640 *ps_bridge = i2c_get_clientdata(client); + + kfree(ps_bridge->edid); + ps_bridge->edid = NULL; +} + static const struct of_device_id ps8640_match[] = { { .compatible = "parade,ps8640" }, { } @@ -775,6 +787,7 @@ MODULE_DEVICE_TABLE(of, ps8640_match); static struct i2c_driver ps8640_driver = { .probe_new = ps8640_probe, + .remove = ps8640_remove, .driver = { .name = "ps8640", .of_match_table = ps8640_match,
When there are multiple EDID reads, the bridge will be repeatedly enabled and disabled. Add a cache for EDID to speed this up. Signed-off-by: Pin-yen Lin <treapking@chromium.org> --- drivers/gpu/drm/bridge/parade-ps8640.c | 61 ++++++++++++++++---------- 1 file changed, 37 insertions(+), 24 deletions(-)