Hi,
On 2023/4/4 22:10, Emil Velikov wrote:
+static void lsdc_crtc_reset(struct drm_crtc *crtc) +{
struct lsdc_display_pipe *dispipe = crtc_to_display_pipe(crtc);struct drm_device *ddev = crtc->dev;struct lsdc_device *ldev = to_lsdc(ddev);struct lsdc_crtc_state *priv_crtc_state;unsigned int index = dispipe->index;u32 val;val = LSDC_PF_XRGB8888 | CFG_RESET_N;if (ldev->descp->chip == CHIP_LS7A2000)val |= LSDC_DMA_STEP_64_BYTES;lsdc_crtc_wreg32(ldev, LSDC_CRTC0_CFG_REG, index, val);if (ldev->descp->chip == CHIP_LS7A2000) {val = PHY_CLOCK_EN | PHY_DATA_EN;lsdc_crtc_wreg32(ldev, LSDC_CRTC0_PANEL_CONF_REG, index, val);}AFAICT no other driver touches the HW in their reset callback. Should the above be moved to another callback?
You may correct in the 95% of all cases.
After reading the comments of the reset callback of struct drm_crtc_funcs in drm_crtc.h,
It seems that it does not prohibit us to touches the hardware.
I copy that comments and paste into here for easier to read,as below:
/* * @reset: * * Reset CRTC hardware and software state to off. This function isn't * called by the core directly, only through drm_mode_config_reset(). * It's not a helper hook only for historical reasons. * * Atomic drivers can use drm_atomic_helper_crtc_reset() to reset * atomic state using this hook. */
It seem allowable to reset CRTC hardware in this callback, did it cue us?
What we know is that this reset callback (and others, such as encoder's reset)
is called by drm_mode_config_reset(). It is the first set of functions get called
before other hardware related callbacks.
I don't not see how other drivers implement this callback, after you mention this
I skim over a few, I found tilcdc also writing the hardware in their tilcdc_crtc_reset()
function. See it in drm/tildc/tilclc_crtc.c
In addition, Loongson platform support efifb, in order to light up the monitor in
firmware stage and the booting stage, the firmware touch the display hardware
register directly. After efifb get kick out, when drm/loongson driver taken over the
hardware, the register setting state still remain in the hardware register. Those
register setting may no longer correct for the subsequent operationd. What we
doing here is to giving the hardware a basic healthy state prepare to be update
further. As the reset callback is call very early, we found that it's the best fit.
The reset will also get called when resume(S3).
The problem is that we don't find a good to place to move those setting currently.