On 3/19/2024 11:15 AM, Doug Anderson wrote:
Hi,
On Tue, Mar 19, 2024 at 10:27 AM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
On Tue, 19 Mar 2024 at 19:13, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
On 3/18/2024 5:55 PM, Dmitry Baryshkov wrote:
On Tue, 19 Mar 2024 at 02:19, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
+bjorn, johan as fyi for sc8280xp
On 3/15/2024 2:36 PM, Douglas Anderson wrote:
Before the introduction of the wait_hpd_asserted() callback in commit
841d742f094e ("drm/dp: Add wait_hpd_asserted() callback to struct
drm_dp_aux") the API between panel drivers and DP AUX bus drivers was
that it was up to the AUX bus driver to wait for HPD in the transfer()
function.
Now wait_hpd_asserted() has been added. The two panel drivers that are
DP AUX endpoints use it. See commit 2327b13d6c47 ("drm/panel-edp: Take
advantage of wait_hpd_asserted() in struct drm_dp_aux") and commit
3b5765df375c ("drm/panel: atna33xc20: Take advantage of
wait_hpd_asserted() in struct drm_dp_aux"). We've implemented
wait_hpd_asserted() in the MSM DP driver as of commit e2969ee30252
("drm/msm/dp: move of_dp_aux_populate_bus() to eDP probe()"). There is
no longer any reason for long wait in the AUX transfer() function.
Remove it.
NOTE: the wait_hpd_asserted() is listed as "optional". That means it's
optional for the DP AUX bus to implement. In the case of the MSM DP
driver we implement it so we can assume it will be called.
How do we enforce that for any new edp panels to be used with MSM, the
panels should atleast invoke wait_hpd_asserted()?
I agree that since MSM implements it, even though its listed as
optional, we can drop this additional wait. So nothing wrong with this
patch for current users including sc8280xp, sc7280 and sc7180.
But, does there need to be some documentation that the edp panels not
using the panel-edp framework should still invoke wait_hpd_asserted()?
Since its marked as optional, what happens if the edp panel driver,
skips calling wait_hpd_asserted()?
It is optional for the DP AUX implementations, not for the panel to be called.
Yes, I understood that part, but is there anything from the panel side
which mandates calling wait_hpd_asserted()?
Is this documented somewhere for all edp panels to do:
if (aux->wait_hpd_asserted)
aux->wait_hpd_asserted(aux, wait_us);
That's obviously not true, e.g. if panel-edp.c handled HPD signal via
the GPIO pin.
But the documentation explicitly says that the host will be powered up
automatically, but the caller must ensure that the panel is powered
on. `It's up to the caller of this code to make sure that the panel is
powered on if getting an error back is not OK.'
It wouldn't hurt to send out a documentation patch that makes this
more explicit. OK, I sent:
https://lore.kernel.org/r/20240319111432.1.I521dad0693cc24fe4dd14cba0c7048d94f5b6b41@changeid
-Doug
Thanks, with that in place, this is
Reviewed-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
