Re: [PATCH v2] drm/bridge: analogix_dp: Ensure the panel is properly prepared/unprepared
On 08/09/2016 08:05 AM, Yakir Yang wrote: + Archit On 08/09/2016 02:53 AM, Sean Paul wrote: Instead of just preparing the panel on bind, actually prepare/unprepare during modeset/disable. The panel must be prepared in order to read hpd status and edid, so we need to keep state around the prepares in order to ensure we don't accidentally turn the panel off at the wrong time. Signed-off-by: Sean PaulReviewed-by: Yakir Yang And I also tested this patch on RK3399 Kevin board, panel works rightly, so: Tested-by: Yakir Yang Also add Archit into CC list, guess this patch should go through his drm_bridge's tree. Reviewed-by: Archit Taneja Thanks, - Yakir --- Changes in v2: - Added panel_is_modeset state/lock to avoid racing detect with modeset (marcheu) - Added prepare/unprepare in .get_modes (yakir) drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 101 ++--- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 32715da..47c449a 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -923,11 +923,63 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) analogix_dp_start_video(dp); } +/* + * This function is a bit of a catch-all for panel preparation, hopefully + * simplifying the logic of functions that need to prepare/unprepare the panel + * below. + * + * If @prepare is true, this function will prepare the panel. Conversely, if it + * is false, the panel will be unprepared. + * + * If @is_modeset_prepare is true, the function will disregard the current state + * of the panel and either prepare/unprepare the panel based on @prepare. Once + * it finishes, it will update dp->panel_is_modeset to reflect the current state + * of the panel. + */ +static int analogix_dp_prepare_panel(struct analogix_dp_device *dp, + bool prepare, bool is_modeset_prepare) +{ +int ret = 0; + +if (!dp->plat_data->panel) +return 0; + +mutex_lock(>panel_lock); + +/* + * Exit early if this is a temporary prepare/unprepare and we're already + * modeset (since we neither want to prepare twice or unprepare early). + */ +if (dp->panel_is_modeset && !is_modeset_prepare) +goto out; + +if (prepare) +ret = drm_panel_prepare(dp->plat_data->panel); +else +ret = drm_panel_unprepare(dp->plat_data->panel); + +if (ret) +goto out; + +if (is_modeset_prepare) +dp->panel_is_modeset = prepare; + +out: +mutex_unlock(>panel_lock); +return ret; +} + int analogix_dp_get_modes(struct drm_connector *connector) { struct analogix_dp_device *dp = to_dp(connector); struct edid *edid = (struct edid *)dp->edid; -int num_modes = 0; +int ret, num_modes = 0; + +ret = analogix_dp_prepare_panel(dp, true, false); +if (ret) { +DRM_ERROR("Failed to prepare panel (%d)\n", ret); +return 0; +} if (analogix_dp_handle_edid(dp) == 0) { drm_mode_connector_update_edid_property(>connector, edid); @@ -940,6 +992,10 @@ int analogix_dp_get_modes(struct drm_connector *connector) if (dp->plat_data->get_modes) num_modes += dp->plat_data->get_modes(dp->plat_data, connector); +ret = analogix_dp_prepare_panel(dp, false, false); +if (ret) +DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + return num_modes; } @@ -960,11 +1016,23 @@ enum drm_connector_status analogix_dp_detect(struct drm_connector *connector, bool force) { struct analogix_dp_device *dp = to_dp(connector); +enum drm_connector_status status = connector_status_disconnected; +int ret; -if (analogix_dp_detect_hpd(dp)) +ret = analogix_dp_prepare_panel(dp, true, false); +if (ret) { +DRM_ERROR("Failed to prepare panel (%d)\n", ret); return connector_status_disconnected; +} + +if (!analogix_dp_detect_hpd(dp)) +status = connector_status_connected; -return connector_status_connected; +ret = analogix_dp_prepare_panel(dp, false, false); +if (ret) +DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + +return status; } static void analogix_dp_connector_destroy(struct drm_connector *connector) @@ -1035,6 +1103,16 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge) return 0; } +static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) +{ +struct analogix_dp_device *dp = bridge->driver_private; +int ret; + +ret = analogix_dp_prepare_panel(dp, true, true); +if (ret) +DRM_ERROR("failed to setup the panel ret = %d\n", ret); +} + static void
Re: [PATCH v2] drm/bridge: analogix_dp: Ensure the panel is properly prepared/unprepared
On 08/09/2016 08:05 AM, Yakir Yang wrote: + Archit On 08/09/2016 02:53 AM, Sean Paul wrote: Instead of just preparing the panel on bind, actually prepare/unprepare during modeset/disable. The panel must be prepared in order to read hpd status and edid, so we need to keep state around the prepares in order to ensure we don't accidentally turn the panel off at the wrong time. Signed-off-by: Sean Paul Reviewed-by: Yakir Yang And I also tested this patch on RK3399 Kevin board, panel works rightly, so: Tested-by: Yakir Yang Also add Archit into CC list, guess this patch should go through his drm_bridge's tree. Reviewed-by: Archit Taneja Thanks, - Yakir --- Changes in v2: - Added panel_is_modeset state/lock to avoid racing detect with modeset (marcheu) - Added prepare/unprepare in .get_modes (yakir) drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 101 ++--- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 32715da..47c449a 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -923,11 +923,63 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) analogix_dp_start_video(dp); } +/* + * This function is a bit of a catch-all for panel preparation, hopefully + * simplifying the logic of functions that need to prepare/unprepare the panel + * below. + * + * If @prepare is true, this function will prepare the panel. Conversely, if it + * is false, the panel will be unprepared. + * + * If @is_modeset_prepare is true, the function will disregard the current state + * of the panel and either prepare/unprepare the panel based on @prepare. Once + * it finishes, it will update dp->panel_is_modeset to reflect the current state + * of the panel. + */ +static int analogix_dp_prepare_panel(struct analogix_dp_device *dp, + bool prepare, bool is_modeset_prepare) +{ +int ret = 0; + +if (!dp->plat_data->panel) +return 0; + +mutex_lock(>panel_lock); + +/* + * Exit early if this is a temporary prepare/unprepare and we're already + * modeset (since we neither want to prepare twice or unprepare early). + */ +if (dp->panel_is_modeset && !is_modeset_prepare) +goto out; + +if (prepare) +ret = drm_panel_prepare(dp->plat_data->panel); +else +ret = drm_panel_unprepare(dp->plat_data->panel); + +if (ret) +goto out; + +if (is_modeset_prepare) +dp->panel_is_modeset = prepare; + +out: +mutex_unlock(>panel_lock); +return ret; +} + int analogix_dp_get_modes(struct drm_connector *connector) { struct analogix_dp_device *dp = to_dp(connector); struct edid *edid = (struct edid *)dp->edid; -int num_modes = 0; +int ret, num_modes = 0; + +ret = analogix_dp_prepare_panel(dp, true, false); +if (ret) { +DRM_ERROR("Failed to prepare panel (%d)\n", ret); +return 0; +} if (analogix_dp_handle_edid(dp) == 0) { drm_mode_connector_update_edid_property(>connector, edid); @@ -940,6 +992,10 @@ int analogix_dp_get_modes(struct drm_connector *connector) if (dp->plat_data->get_modes) num_modes += dp->plat_data->get_modes(dp->plat_data, connector); +ret = analogix_dp_prepare_panel(dp, false, false); +if (ret) +DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + return num_modes; } @@ -960,11 +1016,23 @@ enum drm_connector_status analogix_dp_detect(struct drm_connector *connector, bool force) { struct analogix_dp_device *dp = to_dp(connector); +enum drm_connector_status status = connector_status_disconnected; +int ret; -if (analogix_dp_detect_hpd(dp)) +ret = analogix_dp_prepare_panel(dp, true, false); +if (ret) { +DRM_ERROR("Failed to prepare panel (%d)\n", ret); return connector_status_disconnected; +} + +if (!analogix_dp_detect_hpd(dp)) +status = connector_status_connected; -return connector_status_connected; +ret = analogix_dp_prepare_panel(dp, false, false); +if (ret) +DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + +return status; } static void analogix_dp_connector_destroy(struct drm_connector *connector) @@ -1035,6 +1103,16 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge) return 0; } +static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) +{ +struct analogix_dp_device *dp = bridge->driver_private; +int ret; + +ret = analogix_dp_prepare_panel(dp, true, true); +if (ret) +DRM_ERROR("failed to setup the panel ret = %d\n", ret); +} + static void analogix_dp_bridge_enable(struct drm_bridge *bridge) { struct analogix_dp_device *dp =
Re: [PATCH v2] drm/bridge: analogix_dp: Ensure the panel is properly prepared/unprepared
On Tue, Aug 09, 2016 at 10:35:57AM +0800, Yakir Yang wrote: > + Archit > > > On 08/09/2016 02:53 AM, Sean Paul wrote: > > Instead of just preparing the panel on bind, actually prepare/unprepare > > during modeset/disable. The panel must be prepared in order to read hpd > > status and edid, so we need to keep state around the prepares in order > > to ensure we don't accidentally turn the panel off at the wrong time. > > > > Signed-off-by: Sean Paul> > Reviewed-by: Yakir Yang > > And I also tested this patch on RK3399 Kevin board, panel works rightly, so: > Tested-by: Yakir Yang > > Also add Archit into CC list, guess this patch should go through his > drm_bridge's tree. drm_bridge stuff moved to -misc ;-) -Daniel > > Thanks, > - Yakir > > > --- > > > > Changes in v2: > > - Added panel_is_modeset state/lock to avoid racing detect with modeset > > (marcheu) > > - Added prepare/unprepare in .get_modes (yakir) > > > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 101 > > ++--- > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + > > 2 files changed, 93 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > index 32715da..47c449a 100644 > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > @@ -923,11 +923,63 @@ static void analogix_dp_commit(struct > > analogix_dp_device *dp) > > analogix_dp_start_video(dp); > > } > > +/* > > + * This function is a bit of a catch-all for panel preparation, hopefully > > + * simplifying the logic of functions that need to prepare/unprepare the > > panel > > + * below. > > + * > > + * If @prepare is true, this function will prepare the panel. Conversely, > > if it > > + * is false, the panel will be unprepared. > > + * > > + * If @is_modeset_prepare is true, the function will disregard the current > > state > > + * of the panel and either prepare/unprepare the panel based on @prepare. > > Once > > + * it finishes, it will update dp->panel_is_modeset to reflect the current > > state > > + * of the panel. > > + */ > > +static int analogix_dp_prepare_panel(struct analogix_dp_device *dp, > > +bool prepare, bool is_modeset_prepare) > > +{ > > + int ret = 0; > > + > > + if (!dp->plat_data->panel) > > + return 0; > > + > > + mutex_lock(>panel_lock); > > + > > + /* > > +* Exit early if this is a temporary prepare/unprepare and we're already > > +* modeset (since we neither want to prepare twice or unprepare early). > > +*/ > > + if (dp->panel_is_modeset && !is_modeset_prepare) > > + goto out; > > + > > + if (prepare) > > + ret = drm_panel_prepare(dp->plat_data->panel); > > + else > > + ret = drm_panel_unprepare(dp->plat_data->panel); > > + > > + if (ret) > > + goto out; > > + > > + if (is_modeset_prepare) > > + dp->panel_is_modeset = prepare; > > + > > +out: > > + mutex_unlock(>panel_lock); > > + return ret; > > +} > > + > > int analogix_dp_get_modes(struct drm_connector *connector) > > { > > struct analogix_dp_device *dp = to_dp(connector); > > struct edid *edid = (struct edid *)dp->edid; > > - int num_modes = 0; > > + int ret, num_modes = 0; > > + > > + ret = analogix_dp_prepare_panel(dp, true, false); > > + if (ret) { > > + DRM_ERROR("Failed to prepare panel (%d)\n", ret); > > + return 0; > > + } > > if (analogix_dp_handle_edid(dp) == 0) { > > drm_mode_connector_update_edid_property(>connector, edid); > > @@ -940,6 +992,10 @@ int analogix_dp_get_modes(struct drm_connector > > *connector) > > if (dp->plat_data->get_modes) > > num_modes += dp->plat_data->get_modes(dp->plat_data, connector); > > + ret = analogix_dp_prepare_panel(dp, false, false); > > + if (ret) > > + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); > > + > > return num_modes; > > } > > @@ -960,11 +1016,23 @@ enum drm_connector_status > > analogix_dp_detect(struct drm_connector *connector, bool force) > > { > > struct analogix_dp_device *dp = to_dp(connector); > > + enum drm_connector_status status = connector_status_disconnected; > > + int ret; > > - if (analogix_dp_detect_hpd(dp)) > > + ret = analogix_dp_prepare_panel(dp, true, false); > > + if (ret) { > > + DRM_ERROR("Failed to prepare panel (%d)\n", ret); > > return connector_status_disconnected; > > + } > > + > > + if (!analogix_dp_detect_hpd(dp)) > > + status = connector_status_connected; > > - return connector_status_connected; > > + ret = analogix_dp_prepare_panel(dp, false, false); > > + if (ret) > > + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); > > +
Re: [PATCH v2] drm/bridge: analogix_dp: Ensure the panel is properly prepared/unprepared
On Tue, Aug 09, 2016 at 10:35:57AM +0800, Yakir Yang wrote: > + Archit > > > On 08/09/2016 02:53 AM, Sean Paul wrote: > > Instead of just preparing the panel on bind, actually prepare/unprepare > > during modeset/disable. The panel must be prepared in order to read hpd > > status and edid, so we need to keep state around the prepares in order > > to ensure we don't accidentally turn the panel off at the wrong time. > > > > Signed-off-by: Sean Paul > > Reviewed-by: Yakir Yang > > And I also tested this patch on RK3399 Kevin board, panel works rightly, so: > Tested-by: Yakir Yang > > Also add Archit into CC list, guess this patch should go through his > drm_bridge's tree. drm_bridge stuff moved to -misc ;-) -Daniel > > Thanks, > - Yakir > > > --- > > > > Changes in v2: > > - Added panel_is_modeset state/lock to avoid racing detect with modeset > > (marcheu) > > - Added prepare/unprepare in .get_modes (yakir) > > > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 101 > > ++--- > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + > > 2 files changed, 93 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > index 32715da..47c449a 100644 > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > @@ -923,11 +923,63 @@ static void analogix_dp_commit(struct > > analogix_dp_device *dp) > > analogix_dp_start_video(dp); > > } > > +/* > > + * This function is a bit of a catch-all for panel preparation, hopefully > > + * simplifying the logic of functions that need to prepare/unprepare the > > panel > > + * below. > > + * > > + * If @prepare is true, this function will prepare the panel. Conversely, > > if it > > + * is false, the panel will be unprepared. > > + * > > + * If @is_modeset_prepare is true, the function will disregard the current > > state > > + * of the panel and either prepare/unprepare the panel based on @prepare. > > Once > > + * it finishes, it will update dp->panel_is_modeset to reflect the current > > state > > + * of the panel. > > + */ > > +static int analogix_dp_prepare_panel(struct analogix_dp_device *dp, > > +bool prepare, bool is_modeset_prepare) > > +{ > > + int ret = 0; > > + > > + if (!dp->plat_data->panel) > > + return 0; > > + > > + mutex_lock(>panel_lock); > > + > > + /* > > +* Exit early if this is a temporary prepare/unprepare and we're already > > +* modeset (since we neither want to prepare twice or unprepare early). > > +*/ > > + if (dp->panel_is_modeset && !is_modeset_prepare) > > + goto out; > > + > > + if (prepare) > > + ret = drm_panel_prepare(dp->plat_data->panel); > > + else > > + ret = drm_panel_unprepare(dp->plat_data->panel); > > + > > + if (ret) > > + goto out; > > + > > + if (is_modeset_prepare) > > + dp->panel_is_modeset = prepare; > > + > > +out: > > + mutex_unlock(>panel_lock); > > + return ret; > > +} > > + > > int analogix_dp_get_modes(struct drm_connector *connector) > > { > > struct analogix_dp_device *dp = to_dp(connector); > > struct edid *edid = (struct edid *)dp->edid; > > - int num_modes = 0; > > + int ret, num_modes = 0; > > + > > + ret = analogix_dp_prepare_panel(dp, true, false); > > + if (ret) { > > + DRM_ERROR("Failed to prepare panel (%d)\n", ret); > > + return 0; > > + } > > if (analogix_dp_handle_edid(dp) == 0) { > > drm_mode_connector_update_edid_property(>connector, edid); > > @@ -940,6 +992,10 @@ int analogix_dp_get_modes(struct drm_connector > > *connector) > > if (dp->plat_data->get_modes) > > num_modes += dp->plat_data->get_modes(dp->plat_data, connector); > > + ret = analogix_dp_prepare_panel(dp, false, false); > > + if (ret) > > + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); > > + > > return num_modes; > > } > > @@ -960,11 +1016,23 @@ enum drm_connector_status > > analogix_dp_detect(struct drm_connector *connector, bool force) > > { > > struct analogix_dp_device *dp = to_dp(connector); > > + enum drm_connector_status status = connector_status_disconnected; > > + int ret; > > - if (analogix_dp_detect_hpd(dp)) > > + ret = analogix_dp_prepare_panel(dp, true, false); > > + if (ret) { > > + DRM_ERROR("Failed to prepare panel (%d)\n", ret); > > return connector_status_disconnected; > > + } > > + > > + if (!analogix_dp_detect_hpd(dp)) > > + status = connector_status_connected; > > - return connector_status_connected; > > + ret = analogix_dp_prepare_panel(dp, false, false); > > + if (ret) > > + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); > > + > > + return status; > > } > > static void
Re: [PATCH v2] drm/bridge: analogix_dp: Ensure the panel is properly prepared/unprepared
+ Archit On 08/09/2016 02:53 AM, Sean Paul wrote: Instead of just preparing the panel on bind, actually prepare/unprepare during modeset/disable. The panel must be prepared in order to read hpd status and edid, so we need to keep state around the prepares in order to ensure we don't accidentally turn the panel off at the wrong time. Signed-off-by: Sean PaulReviewed-by: Yakir Yang And I also tested this patch on RK3399 Kevin board, panel works rightly, so: Tested-by: Yakir Yang Also add Archit into CC list, guess this patch should go through his drm_bridge's tree. Thanks, - Yakir --- Changes in v2: - Added panel_is_modeset state/lock to avoid racing detect with modeset (marcheu) - Added prepare/unprepare in .get_modes (yakir) drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 101 ++--- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 32715da..47c449a 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -923,11 +923,63 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) analogix_dp_start_video(dp); } +/* + * This function is a bit of a catch-all for panel preparation, hopefully + * simplifying the logic of functions that need to prepare/unprepare the panel + * below. + * + * If @prepare is true, this function will prepare the panel. Conversely, if it + * is false, the panel will be unprepared. + * + * If @is_modeset_prepare is true, the function will disregard the current state + * of the panel and either prepare/unprepare the panel based on @prepare. Once + * it finishes, it will update dp->panel_is_modeset to reflect the current state + * of the panel. + */ +static int analogix_dp_prepare_panel(struct analogix_dp_device *dp, +bool prepare, bool is_modeset_prepare) +{ + int ret = 0; + + if (!dp->plat_data->panel) + return 0; + + mutex_lock(>panel_lock); + + /* +* Exit early if this is a temporary prepare/unprepare and we're already +* modeset (since we neither want to prepare twice or unprepare early). +*/ + if (dp->panel_is_modeset && !is_modeset_prepare) + goto out; + + if (prepare) + ret = drm_panel_prepare(dp->plat_data->panel); + else + ret = drm_panel_unprepare(dp->plat_data->panel); + + if (ret) + goto out; + + if (is_modeset_prepare) + dp->panel_is_modeset = prepare; + +out: + mutex_unlock(>panel_lock); + return ret; +} + int analogix_dp_get_modes(struct drm_connector *connector) { struct analogix_dp_device *dp = to_dp(connector); struct edid *edid = (struct edid *)dp->edid; - int num_modes = 0; + int ret, num_modes = 0; + + ret = analogix_dp_prepare_panel(dp, true, false); + if (ret) { + DRM_ERROR("Failed to prepare panel (%d)\n", ret); + return 0; + } if (analogix_dp_handle_edid(dp) == 0) { drm_mode_connector_update_edid_property(>connector, edid); @@ -940,6 +992,10 @@ int analogix_dp_get_modes(struct drm_connector *connector) if (dp->plat_data->get_modes) num_modes += dp->plat_data->get_modes(dp->plat_data, connector); + ret = analogix_dp_prepare_panel(dp, false, false); + if (ret) + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + return num_modes; } @@ -960,11 +1016,23 @@ enum drm_connector_status analogix_dp_detect(struct drm_connector *connector, bool force) { struct analogix_dp_device *dp = to_dp(connector); + enum drm_connector_status status = connector_status_disconnected; + int ret; - if (analogix_dp_detect_hpd(dp)) + ret = analogix_dp_prepare_panel(dp, true, false); + if (ret) { + DRM_ERROR("Failed to prepare panel (%d)\n", ret); return connector_status_disconnected; + } + + if (!analogix_dp_detect_hpd(dp)) + status = connector_status_connected; - return connector_status_connected; + ret = analogix_dp_prepare_panel(dp, false, false); + if (ret) + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + + return status; } static void analogix_dp_connector_destroy(struct drm_connector *connector) @@ -1035,6 +1103,16 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge) return 0; } +static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) +{ + struct analogix_dp_device *dp = bridge->driver_private; + int ret; + + ret =
Re: [PATCH v2] drm/bridge: analogix_dp: Ensure the panel is properly prepared/unprepared
+ Archit On 08/09/2016 02:53 AM, Sean Paul wrote: Instead of just preparing the panel on bind, actually prepare/unprepare during modeset/disable. The panel must be prepared in order to read hpd status and edid, so we need to keep state around the prepares in order to ensure we don't accidentally turn the panel off at the wrong time. Signed-off-by: Sean Paul Reviewed-by: Yakir Yang And I also tested this patch on RK3399 Kevin board, panel works rightly, so: Tested-by: Yakir Yang Also add Archit into CC list, guess this patch should go through his drm_bridge's tree. Thanks, - Yakir --- Changes in v2: - Added panel_is_modeset state/lock to avoid racing detect with modeset (marcheu) - Added prepare/unprepare in .get_modes (yakir) drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 101 ++--- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 32715da..47c449a 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -923,11 +923,63 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) analogix_dp_start_video(dp); } +/* + * This function is a bit of a catch-all for panel preparation, hopefully + * simplifying the logic of functions that need to prepare/unprepare the panel + * below. + * + * If @prepare is true, this function will prepare the panel. Conversely, if it + * is false, the panel will be unprepared. + * + * If @is_modeset_prepare is true, the function will disregard the current state + * of the panel and either prepare/unprepare the panel based on @prepare. Once + * it finishes, it will update dp->panel_is_modeset to reflect the current state + * of the panel. + */ +static int analogix_dp_prepare_panel(struct analogix_dp_device *dp, +bool prepare, bool is_modeset_prepare) +{ + int ret = 0; + + if (!dp->plat_data->panel) + return 0; + + mutex_lock(>panel_lock); + + /* +* Exit early if this is a temporary prepare/unprepare and we're already +* modeset (since we neither want to prepare twice or unprepare early). +*/ + if (dp->panel_is_modeset && !is_modeset_prepare) + goto out; + + if (prepare) + ret = drm_panel_prepare(dp->plat_data->panel); + else + ret = drm_panel_unprepare(dp->plat_data->panel); + + if (ret) + goto out; + + if (is_modeset_prepare) + dp->panel_is_modeset = prepare; + +out: + mutex_unlock(>panel_lock); + return ret; +} + int analogix_dp_get_modes(struct drm_connector *connector) { struct analogix_dp_device *dp = to_dp(connector); struct edid *edid = (struct edid *)dp->edid; - int num_modes = 0; + int ret, num_modes = 0; + + ret = analogix_dp_prepare_panel(dp, true, false); + if (ret) { + DRM_ERROR("Failed to prepare panel (%d)\n", ret); + return 0; + } if (analogix_dp_handle_edid(dp) == 0) { drm_mode_connector_update_edid_property(>connector, edid); @@ -940,6 +992,10 @@ int analogix_dp_get_modes(struct drm_connector *connector) if (dp->plat_data->get_modes) num_modes += dp->plat_data->get_modes(dp->plat_data, connector); + ret = analogix_dp_prepare_panel(dp, false, false); + if (ret) + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + return num_modes; } @@ -960,11 +1016,23 @@ enum drm_connector_status analogix_dp_detect(struct drm_connector *connector, bool force) { struct analogix_dp_device *dp = to_dp(connector); + enum drm_connector_status status = connector_status_disconnected; + int ret; - if (analogix_dp_detect_hpd(dp)) + ret = analogix_dp_prepare_panel(dp, true, false); + if (ret) { + DRM_ERROR("Failed to prepare panel (%d)\n", ret); return connector_status_disconnected; + } + + if (!analogix_dp_detect_hpd(dp)) + status = connector_status_connected; - return connector_status_connected; + ret = analogix_dp_prepare_panel(dp, false, false); + if (ret) + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + + return status; } static void analogix_dp_connector_destroy(struct drm_connector *connector) @@ -1035,6 +1103,16 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge) return 0; } +static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) +{ + struct analogix_dp_device *dp = bridge->driver_private; + int ret; + + ret = analogix_dp_prepare_panel(dp, true, true); + if (ret) +
[PATCH v2] drm/bridge: analogix_dp: Ensure the panel is properly prepared/unprepared
Instead of just preparing the panel on bind, actually prepare/unprepare during modeset/disable. The panel must be prepared in order to read hpd status and edid, so we need to keep state around the prepares in order to ensure we don't accidentally turn the panel off at the wrong time. Signed-off-by: Sean Paul--- Changes in v2: - Added panel_is_modeset state/lock to avoid racing detect with modeset (marcheu) - Added prepare/unprepare in .get_modes (yakir) drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 101 ++--- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 32715da..47c449a 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -923,11 +923,63 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) analogix_dp_start_video(dp); } +/* + * This function is a bit of a catch-all for panel preparation, hopefully + * simplifying the logic of functions that need to prepare/unprepare the panel + * below. + * + * If @prepare is true, this function will prepare the panel. Conversely, if it + * is false, the panel will be unprepared. + * + * If @is_modeset_prepare is true, the function will disregard the current state + * of the panel and either prepare/unprepare the panel based on @prepare. Once + * it finishes, it will update dp->panel_is_modeset to reflect the current state + * of the panel. + */ +static int analogix_dp_prepare_panel(struct analogix_dp_device *dp, +bool prepare, bool is_modeset_prepare) +{ + int ret = 0; + + if (!dp->plat_data->panel) + return 0; + + mutex_lock(>panel_lock); + + /* +* Exit early if this is a temporary prepare/unprepare and we're already +* modeset (since we neither want to prepare twice or unprepare early). +*/ + if (dp->panel_is_modeset && !is_modeset_prepare) + goto out; + + if (prepare) + ret = drm_panel_prepare(dp->plat_data->panel); + else + ret = drm_panel_unprepare(dp->plat_data->panel); + + if (ret) + goto out; + + if (is_modeset_prepare) + dp->panel_is_modeset = prepare; + +out: + mutex_unlock(>panel_lock); + return ret; +} + int analogix_dp_get_modes(struct drm_connector *connector) { struct analogix_dp_device *dp = to_dp(connector); struct edid *edid = (struct edid *)dp->edid; - int num_modes = 0; + int ret, num_modes = 0; + + ret = analogix_dp_prepare_panel(dp, true, false); + if (ret) { + DRM_ERROR("Failed to prepare panel (%d)\n", ret); + return 0; + } if (analogix_dp_handle_edid(dp) == 0) { drm_mode_connector_update_edid_property(>connector, edid); @@ -940,6 +992,10 @@ int analogix_dp_get_modes(struct drm_connector *connector) if (dp->plat_data->get_modes) num_modes += dp->plat_data->get_modes(dp->plat_data, connector); + ret = analogix_dp_prepare_panel(dp, false, false); + if (ret) + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + return num_modes; } @@ -960,11 +1016,23 @@ enum drm_connector_status analogix_dp_detect(struct drm_connector *connector, bool force) { struct analogix_dp_device *dp = to_dp(connector); + enum drm_connector_status status = connector_status_disconnected; + int ret; - if (analogix_dp_detect_hpd(dp)) + ret = analogix_dp_prepare_panel(dp, true, false); + if (ret) { + DRM_ERROR("Failed to prepare panel (%d)\n", ret); return connector_status_disconnected; + } + + if (!analogix_dp_detect_hpd(dp)) + status = connector_status_connected; - return connector_status_connected; + ret = analogix_dp_prepare_panel(dp, false, false); + if (ret) + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + + return status; } static void analogix_dp_connector_destroy(struct drm_connector *connector) @@ -1035,6 +1103,16 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge) return 0; } +static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) +{ + struct analogix_dp_device *dp = bridge->driver_private; + int ret; + + ret = analogix_dp_prepare_panel(dp, true, true); + if (ret) + DRM_ERROR("failed to setup the panel ret = %d\n", ret); +} + static void analogix_dp_bridge_enable(struct drm_bridge *bridge) { struct analogix_dp_device *dp = bridge->driver_private; @@ -1058,6 +1136,7 @@ static void analogix_dp_bridge_enable(struct drm_bridge
[PATCH v2] drm/bridge: analogix_dp: Ensure the panel is properly prepared/unprepared
Instead of just preparing the panel on bind, actually prepare/unprepare during modeset/disable. The panel must be prepared in order to read hpd status and edid, so we need to keep state around the prepares in order to ensure we don't accidentally turn the panel off at the wrong time. Signed-off-by: Sean Paul --- Changes in v2: - Added panel_is_modeset state/lock to avoid racing detect with modeset (marcheu) - Added prepare/unprepare in .get_modes (yakir) drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 101 ++--- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 32715da..47c449a 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -923,11 +923,63 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) analogix_dp_start_video(dp); } +/* + * This function is a bit of a catch-all for panel preparation, hopefully + * simplifying the logic of functions that need to prepare/unprepare the panel + * below. + * + * If @prepare is true, this function will prepare the panel. Conversely, if it + * is false, the panel will be unprepared. + * + * If @is_modeset_prepare is true, the function will disregard the current state + * of the panel and either prepare/unprepare the panel based on @prepare. Once + * it finishes, it will update dp->panel_is_modeset to reflect the current state + * of the panel. + */ +static int analogix_dp_prepare_panel(struct analogix_dp_device *dp, +bool prepare, bool is_modeset_prepare) +{ + int ret = 0; + + if (!dp->plat_data->panel) + return 0; + + mutex_lock(>panel_lock); + + /* +* Exit early if this is a temporary prepare/unprepare and we're already +* modeset (since we neither want to prepare twice or unprepare early). +*/ + if (dp->panel_is_modeset && !is_modeset_prepare) + goto out; + + if (prepare) + ret = drm_panel_prepare(dp->plat_data->panel); + else + ret = drm_panel_unprepare(dp->plat_data->panel); + + if (ret) + goto out; + + if (is_modeset_prepare) + dp->panel_is_modeset = prepare; + +out: + mutex_unlock(>panel_lock); + return ret; +} + int analogix_dp_get_modes(struct drm_connector *connector) { struct analogix_dp_device *dp = to_dp(connector); struct edid *edid = (struct edid *)dp->edid; - int num_modes = 0; + int ret, num_modes = 0; + + ret = analogix_dp_prepare_panel(dp, true, false); + if (ret) { + DRM_ERROR("Failed to prepare panel (%d)\n", ret); + return 0; + } if (analogix_dp_handle_edid(dp) == 0) { drm_mode_connector_update_edid_property(>connector, edid); @@ -940,6 +992,10 @@ int analogix_dp_get_modes(struct drm_connector *connector) if (dp->plat_data->get_modes) num_modes += dp->plat_data->get_modes(dp->plat_data, connector); + ret = analogix_dp_prepare_panel(dp, false, false); + if (ret) + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + return num_modes; } @@ -960,11 +1016,23 @@ enum drm_connector_status analogix_dp_detect(struct drm_connector *connector, bool force) { struct analogix_dp_device *dp = to_dp(connector); + enum drm_connector_status status = connector_status_disconnected; + int ret; - if (analogix_dp_detect_hpd(dp)) + ret = analogix_dp_prepare_panel(dp, true, false); + if (ret) { + DRM_ERROR("Failed to prepare panel (%d)\n", ret); return connector_status_disconnected; + } + + if (!analogix_dp_detect_hpd(dp)) + status = connector_status_connected; - return connector_status_connected; + ret = analogix_dp_prepare_panel(dp, false, false); + if (ret) + DRM_ERROR("Failed to unprepare panel (%d)\n", ret); + + return status; } static void analogix_dp_connector_destroy(struct drm_connector *connector) @@ -1035,6 +1103,16 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge) return 0; } +static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) +{ + struct analogix_dp_device *dp = bridge->driver_private; + int ret; + + ret = analogix_dp_prepare_panel(dp, true, true); + if (ret) + DRM_ERROR("failed to setup the panel ret = %d\n", ret); +} + static void analogix_dp_bridge_enable(struct drm_bridge *bridge) { struct analogix_dp_device *dp = bridge->driver_private; @@ -1058,6 +1136,7 @@ static void analogix_dp_bridge_enable(struct drm_bridge *bridge) static void