Re: [Freedreno] [PATCH v4 1/1] drm: allow limiting the scatter list size.
On Tue, Sep 08, 2020 at 07:48:58AM +0200, Gerd Hoffmann wrote: > On Mon, Sep 07, 2020 at 03:53:02PM +0200, Daniel Vetter wrote: > > On Mon, Sep 7, 2020 at 1:24 PM Gerd Hoffmann wrote: > > > > > > Add drm_device argument to drm_prime_pages_to_sg(), so we can > > > call dma_max_mapping_size() to figure the segment size limit > > > and call into __sg_alloc_table_from_pages() with the correct > > > limit. > > > > > > This fixes virtio-gpu with sev. Possibly it'll fix other bugs > > > too given that drm seems to totaly ignore segment size limits > > > so far ... > > > > > > v2: place max_segment in drm driver not gem object. > > > v3: move max_segment next to the other gem fields. > > > v4: just use dma_max_mapping_size(). > > > > > > Signed-off-by: Gerd Hoffmann > > > > Uh, are you sure this works in all cases for virtio? > > Sure, I've tested it ;) > > > The comments I've found suggest very much not ... Or is that all very > > old stuff only that no one cares about anymore? > > I think these days it is possible to override dma_ops per device, which > in turn allows virtio to deal with the quirks without the rest of the > kernel knowing about these details. > > I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just > use the dma api path unconditionally and depend on virtio core having > setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. The comment above vring_use_dma_api() suggests that this has not yet happened, that's why I'm asking. If this has happened then I think it'd be best if you remove that todo entry and update it, as part of the overall series to add dma_max_mapping_size and remove the quirks. Otherwise this all is a bit wtf material :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v4 1/1] drm: allow limiting the scatter list size.
> > > The comments I've found suggest very much not ... Or is that all very > > > old stuff only that no one cares about anymore? > > > > I think these days it is possible to override dma_ops per device, which > > in turn allows virtio to deal with the quirks without the rest of the > > kernel knowing about these details. > > > > I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just > > use the dma api path unconditionally and depend on virtio core having > > setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. > > The comment above vring_use_dma_api() suggests that this has not yet > happened, that's why I'm asking. Hmm, wading through the code, seems it indeed happen yet, even though my testing didn't show any issues. Probably pure luck because devices and cpus have the same memory view on x86. Guess I need to try this on ppc64 to see it actually failing ... So dropping the virtio_has_dma_quirk() checks isn't going to fly. Using dma_max_mapping_size() should be fine though. It might use a lower limit than needed for virtio, but it should not break things. take care, Gerd ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v4 1/1] drm: allow limiting the scatter list size.
On Tue, Sep 08, 2020 at 12:02:53PM +0200, Gerd Hoffmann wrote: > > > > The comments I've found suggest very much not ... Or is that all very > > > > old stuff only that no one cares about anymore? > > > > > > I think these days it is possible to override dma_ops per device, which > > > in turn allows virtio to deal with the quirks without the rest of the > > > kernel knowing about these details. > > > > > > I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just > > > use the dma api path unconditionally and depend on virtio core having > > > setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. > > > > The comment above vring_use_dma_api() suggests that this has not yet > > happened, that's why I'm asking. > > Hmm, wading through the code, seems it indeed happen yet, even though my > testing didn't show any issues. Probably pure luck because devices and > cpus have the same memory view on x86. Guess I need to try this on > ppc64 to see it actually failing ... > > So dropping the virtio_has_dma_quirk() checks isn't going to fly. > > Using dma_max_mapping_size() should be fine though. It might use a > lower limit than needed for virtio, but it should not break things. Makes sense. On this patch here: Reviewed-by: Daniel Vetter And I guess would be good if virtio pushes a bit more towards using the dma api abstraction fully so we can get rid of these hacks. Virtio feels like a driver that really should be using dma-api and not dig around behind it because "it' makes stuff 0.5% faster" or so, since being virtualized it's already not the king of speed anyway :-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] Adreno GPU microcode update
The following changes since commit d5f9eea5a251d43412b07f5295d03e97b89ac4a5: wl18xx: update firmware file 8.9.0.0.83 (2020-09-01 08:07:59 -0400) are available in the Git repository at: https://github.com/CosmicPenguin/linux-firmware.git for-master for you to fetch changes up to f48fec44127f88ce83ea1bcaf5824de4146ca2f9: qcom: Add updated a5xx and a6xx microcode (2020-09-08 10:03:36 -0600) Jordan Crouse (1): qcom: Add updated a5xx and a6xx microcode qcom/a530_pfp.fw | Bin 15876 -> 16144 bytes qcom/a630_sqe.fw | Bin 31316 -> 32056 bytes 2 files changed, 0 insertions(+), 0 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v5] drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets
Quoting Kuogee Hsieh (2020-09-04 12:54:39) > add event thread to execute events serially from event queue. Also > timeout mode is supported which allow an event be deferred to be > executed at later time. Both link and phy compliant tests had been > done successfully. > > Changes in v2: > - Fix potential deadlock by removing redundant connect_mutex > - Check and enable link clock during modeset > - Drop unused code and fix function prototypes. > - set sink power to normal operation state (D0) before DPCD read > > Changes in v3: > - push idle pattern at main link before timing generator off > - add timeout handles for both connect and disconnect > > Changes in v4: > - add ST_SUSPEND_PENDING to handles suspend/modeset test operations > - clear dp phy aux interrupt status when ERR_DPPHY_AUX error > - send segment addr during edid read > - clear bpp depth before MISC register write > > Changes in v5: > - add ST_SUSPENDED to fix crash at resume > > Signed-off-by: Kuogee Hsieh > > This patch depends-on following series: > https://lore.kernel.org/dri-devel/20200818051137.21478-1-tan...@codeaurora.org/ Can this be based on v12 of the patch series? v4 of this patch was based on v12, but this has regressed and gone back to v11. v12 is https://lkml.kernel.org/lkml/20200827211658.27479-1-tan...@codeaurora.org/ ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: move vblank events to complete_commit()
On 2020-09-07 10:04, Rob Clark wrote: From: Rob Clark We could get a vblank event racing with the current atomic commit, resulting in sending the pageflip event to userspace early, causing tearing. On the other hand, complete_commit() ensures that the pending flush is complete. Signed-off-by: Rob Clark I checked our downstream code as well and yes we are not signaling page flips inside the vblank_cb and are doing it after wait_for_commit_done This aligns with that. Hence, Reviewed-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index c2729f71e2fa..89c0245b5de5 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -297,7 +297,6 @@ void dpu_crtc_vblank_callback(struct drm_crtc *crtc) dpu_crtc->vblank_cb_time = ktime_get(); else dpu_crtc->vblank_cb_count++; - _dpu_crtc_complete_flip(crtc); drm_crtc_handle_vblank(crtc); trace_dpu_crtc_vblank_cb(DRMID(crtc)); } @@ -402,6 +401,7 @@ static void dpu_crtc_frame_event_cb(void *data, u32 event) void dpu_crtc_complete_commit(struct drm_crtc *crtc) { trace_dpu_crtc_complete_commit(DRMID(crtc)); + _dpu_crtc_complete_flip(crtc); } static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc, ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 2/2] drm/msm/dpu: clean up some impossibilities
On 2020-09-07 10:04, Rob Clark wrote: From: Rob Clark Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 81 1 file changed, 12 insertions(+), 69 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 89c0245b5de5..ad49b0e17fcb 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -265,11 +265,6 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct drm_crtc *crtc) { struct drm_encoder *encoder; - if (!crtc) { - DPU_ERROR("invalid crtc\n"); - return INTF_MODE_NONE; - } - /* * TODO: This function is called from dpu debugfs and as part of atomic * check. When called from debugfs, the crtc->mutex must be held to @@ -500,12 +495,6 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc, struct dpu_crtc_state *cstate; struct drm_encoder *encoder; struct drm_device *dev; - unsigned long flags; - - if (!crtc) { - DPU_ERROR("invalid crtc\n"); - return; - } if (!crtc->state->enable) { DPU_DEBUG("crtc%d -> enable %d, skip atomic_begin\n", @@ -521,15 +510,6 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc, _dpu_crtc_setup_lm_bounds(crtc, crtc->state); - if (dpu_crtc->event) { - WARN_ON(dpu_crtc->event); - } else { - spin_lock_irqsave(&dev->event_lock, flags); - dpu_crtc->event = crtc->state->event; - crtc->state->event = NULL; - spin_unlock_irqrestore(&dev->event_lock, flags); - } - /* encoder will trigger pending mask now */ drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask) dpu_encoder_trigger_kickoff_pending(encoder); @@ -583,14 +563,11 @@ static void dpu_crtc_atomic_flush(struct drm_crtc *crtc, return; } - if (dpu_crtc->event) { - DPU_DEBUG("already received dpu_crtc->event\n"); - } else { - spin_lock_irqsave(&dev->event_lock, flags); - dpu_crtc->event = crtc->state->event; - crtc->state->event = NULL; - spin_unlock_irqrestore(&dev->event_lock, flags); - } + WARN_ON(dpu_crtc->event); before the patch "move vblank events to complete_commit()", the dpu_crtc events were consumed in the _dpu_crtc_complete_flip(). So there was a chance that if the vblank event did not come in time before the next commit, dpu_crtc->event will not be consumed. But after the patch, _dpu_crtc_complete_flip() is being signaled in dpu_crtc_complete_commit() which will always happen, so is there any case where we will hit this warning at all or will this catch some other condition? + spin_lock_irqsave(&dev->event_lock, flags); + dpu_crtc->event = crtc->state->event; + crtc->state->event = NULL; + spin_unlock_irqrestore(&dev->event_lock, flags); /* * If no mixers has been allocated in dpu_crtc_atomic_check(), @@ -635,14 +612,7 @@ static void dpu_crtc_atomic_flush(struct drm_crtc *crtc, static void dpu_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state) { - struct dpu_crtc_state *cstate; - - if (!crtc || !state) { - DPU_ERROR("invalid argument(s)\n"); - return; - } - - cstate = to_dpu_crtc_state(state); + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); DPU_DEBUG("crtc%d\n", crtc->base.id); @@ -731,14 +701,8 @@ static void dpu_crtc_reset(struct drm_crtc *crtc) */ static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc *crtc) { - struct dpu_crtc_state *cstate, *old_cstate; - - if (!crtc || !crtc->state) { - DPU_ERROR("invalid argument(s)\n"); - return NULL; - } + struct dpu_crtc_state *cstate, *old_cstate = to_dpu_crtc_state(crtc->state); - old_cstate = to_dpu_crtc_state(crtc->state); cstate = kmemdup(old_cstate, sizeof(*old_cstate), GFP_KERNEL); if (!cstate) { DPU_ERROR("failed to allocate state\n"); @@ -754,19 +718,12 @@ static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc *crtc) static void dpu_crtc_disable(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { - struct dpu_crtc *dpu_crtc; - struct dpu_crtc_state *cstate; + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); struct drm_encoder *encoder; unsigned long flags; bool release_bandwidth = false; - if (!crtc || !crtc->state) { - DPU_ERROR("invalid crtc\n"); - return; - } - dpu_crtc = to_dpu_crtc(crt
Re: [Freedreno] [PATCH 2/2] drm/msm/dpu: clean up some impossibilities
On Tue, Sep 8, 2020 at 12:44 PM wrote: > > On 2020-09-07 10:04, Rob Clark wrote: > > From: Rob Clark > > > > Signed-off-by: Rob Clark > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 81 > > 1 file changed, 12 insertions(+), 69 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > index 89c0245b5de5..ad49b0e17fcb 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > @@ -265,11 +265,6 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct > > drm_crtc *crtc) > > { > > struct drm_encoder *encoder; > > > > - if (!crtc) { > > - DPU_ERROR("invalid crtc\n"); > > - return INTF_MODE_NONE; > > - } > > - > > /* > >* TODO: This function is called from dpu debugfs and as part of > > atomic > >* check. When called from debugfs, the crtc->mutex must be held to > > @@ -500,12 +495,6 @@ static void dpu_crtc_atomic_begin(struct drm_crtc > > *crtc, > > struct dpu_crtc_state *cstate; > > struct drm_encoder *encoder; > > struct drm_device *dev; > > - unsigned long flags; > > - > > - if (!crtc) { > > - DPU_ERROR("invalid crtc\n"); > > - return; > > - } > > > > if (!crtc->state->enable) { > > DPU_DEBUG("crtc%d -> enable %d, skip atomic_begin\n", > > @@ -521,15 +510,6 @@ static void dpu_crtc_atomic_begin(struct drm_crtc > > *crtc, > > > > _dpu_crtc_setup_lm_bounds(crtc, crtc->state); > > > > - if (dpu_crtc->event) { > > - WARN_ON(dpu_crtc->event); > > - } else { > > - spin_lock_irqsave(&dev->event_lock, flags); > > - dpu_crtc->event = crtc->state->event; > > - crtc->state->event = NULL; > > - spin_unlock_irqrestore(&dev->event_lock, flags); > > - } > > - > > /* encoder will trigger pending mask now */ > > drm_for_each_encoder_mask(encoder, crtc->dev, > > crtc->state->encoder_mask) > > dpu_encoder_trigger_kickoff_pending(encoder); > > @@ -583,14 +563,11 @@ static void dpu_crtc_atomic_flush(struct drm_crtc > > *crtc, > > return; > > } > > > > - if (dpu_crtc->event) { > > - DPU_DEBUG("already received dpu_crtc->event\n"); > > - } else { > > - spin_lock_irqsave(&dev->event_lock, flags); > > - dpu_crtc->event = crtc->state->event; > > - crtc->state->event = NULL; > > - spin_unlock_irqrestore(&dev->event_lock, flags); > > - } > > + WARN_ON(dpu_crtc->event); > before the patch "move vblank events to complete_commit()", the dpu_crtc > events > were consumed in the _dpu_crtc_complete_flip(). So there was a chance > that if the vblank event > did not come in time before the next commit, dpu_crtc->event will not be > consumed. > > But after the patch, _dpu_crtc_complete_flip() is being signaled in > dpu_crtc_complete_commit() > which will always happen, so is there any case where we will hit this > warning at all or will this > catch some other condition? The core drm-atomic bits will reject an incoming atomic update if the previous one has not completed, so other than a coding bug, this WARN_ON() should not be hit.. it's only purpose is to make it very obvious if someone breaks something :-) BR, -R > > > + spin_lock_irqsave(&dev->event_lock, flags); > > + dpu_crtc->event = crtc->state->event; > > + crtc->state->event = NULL; > > + spin_unlock_irqrestore(&dev->event_lock, flags); > > > > /* > >* If no mixers has been allocated in dpu_crtc_atomic_check(), > > @@ -635,14 +612,7 @@ static void dpu_crtc_atomic_flush(struct drm_crtc > > *crtc, > > static void dpu_crtc_destroy_state(struct drm_crtc *crtc, > > struct drm_crtc_state *state) > > { > > - struct dpu_crtc_state *cstate; > > - > > - if (!crtc || !state) { > > - DPU_ERROR("invalid argument(s)\n"); > > - return; > > - } > > - > > - cstate = to_dpu_crtc_state(state); > > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); > > > > DPU_DEBUG("crtc%d\n", crtc->base.id); > > > > @@ -731,14 +701,8 @@ static void dpu_crtc_reset(struct drm_crtc *crtc) > > */ > > static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc > > *crtc) > > { > > - struct dpu_crtc_state *cstate, *old_cstate; > > - > > - if (!crtc || !crtc->state) { > > - DPU_ERROR("invalid argument(s)\n"); > > - return NULL; > > - } > > + struct dpu_crtc_state *cstate, *old_cstate = > > to_dpu_crtc_state(crtc->state); > > > > - old_cstate = to_dpu_crtc_state(crtc->state); > > cstate = kmemdup(old_cstate, sizeof(*old_cstate), GFP_KERNEL); > > if (!cstate) { > > DPU_ERROR("failed to allocate state\n"); > > @@ -754,19 +718,12 @@ static struct drm_crtc_state >
Re: [Freedreno] [PATCH 2/2] drm/msm/dpu: clean up some impossibilities
On 2020-09-08 13:11, Rob Clark wrote: On Tue, Sep 8, 2020 at 12:44 PM wrote: On 2020-09-07 10:04, Rob Clark wrote: > From: Rob Clark > > Signed-off-by: Rob Clark Reviewed-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 81 > 1 file changed, 12 insertions(+), 69 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 89c0245b5de5..ad49b0e17fcb 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -265,11 +265,6 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct > drm_crtc *crtc) > { > struct drm_encoder *encoder; > > - if (!crtc) { > - DPU_ERROR("invalid crtc\n"); > - return INTF_MODE_NONE; > - } > - > /* >* TODO: This function is called from dpu debugfs and as part of > atomic >* check. When called from debugfs, the crtc->mutex must be held to > @@ -500,12 +495,6 @@ static void dpu_crtc_atomic_begin(struct drm_crtc > *crtc, > struct dpu_crtc_state *cstate; > struct drm_encoder *encoder; > struct drm_device *dev; > - unsigned long flags; > - > - if (!crtc) { > - DPU_ERROR("invalid crtc\n"); > - return; > - } > > if (!crtc->state->enable) { > DPU_DEBUG("crtc%d -> enable %d, skip atomic_begin\n", > @@ -521,15 +510,6 @@ static void dpu_crtc_atomic_begin(struct drm_crtc > *crtc, > > _dpu_crtc_setup_lm_bounds(crtc, crtc->state); > > - if (dpu_crtc->event) { > - WARN_ON(dpu_crtc->event); > - } else { > - spin_lock_irqsave(&dev->event_lock, flags); > - dpu_crtc->event = crtc->state->event; > - crtc->state->event = NULL; > - spin_unlock_irqrestore(&dev->event_lock, flags); > - } > - > /* encoder will trigger pending mask now */ > drm_for_each_encoder_mask(encoder, crtc->dev, > crtc->state->encoder_mask) > dpu_encoder_trigger_kickoff_pending(encoder); > @@ -583,14 +563,11 @@ static void dpu_crtc_atomic_flush(struct drm_crtc > *crtc, > return; > } > > - if (dpu_crtc->event) { > - DPU_DEBUG("already received dpu_crtc->event\n"); > - } else { > - spin_lock_irqsave(&dev->event_lock, flags); > - dpu_crtc->event = crtc->state->event; > - crtc->state->event = NULL; > - spin_unlock_irqrestore(&dev->event_lock, flags); > - } > + WARN_ON(dpu_crtc->event); before the patch "move vblank events to complete_commit()", the dpu_crtc events were consumed in the _dpu_crtc_complete_flip(). So there was a chance that if the vblank event did not come in time before the next commit, dpu_crtc->event will not be consumed. But after the patch, _dpu_crtc_complete_flip() is being signaled in dpu_crtc_complete_commit() which will always happen, so is there any case where we will hit this warning at all or will this catch some other condition? The core drm-atomic bits will reject an incoming atomic update if the previous one has not completed, so other than a coding bug, this WARN_ON() should not be hit.. it's only purpose is to make it very obvious if someone breaks something :-) BR, -R > + spin_lock_irqsave(&dev->event_lock, flags); > + dpu_crtc->event = crtc->state->event; > + crtc->state->event = NULL; > + spin_unlock_irqrestore(&dev->event_lock, flags); > > /* >* If no mixers has been allocated in dpu_crtc_atomic_check(), > @@ -635,14 +612,7 @@ static void dpu_crtc_atomic_flush(struct drm_crtc > *crtc, > static void dpu_crtc_destroy_state(struct drm_crtc *crtc, > struct drm_crtc_state *state) > { > - struct dpu_crtc_state *cstate; > - > - if (!crtc || !state) { > - DPU_ERROR("invalid argument(s)\n"); > - return; > - } > - > - cstate = to_dpu_crtc_state(state); > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state); > > DPU_DEBUG("crtc%d\n", crtc->base.id); > > @@ -731,14 +701,8 @@ static void dpu_crtc_reset(struct drm_crtc *crtc) > */ > static struct drm_crtc_state *dpu_crtc_duplicate_state(struct drm_crtc > *crtc) > { > - struct dpu_crtc_state *cstate, *old_cstate; > - > - if (!crtc || !crtc->state) { > - DPU_ERROR("invalid argument(s)\n"); > - return NULL; > - } > + struct dpu_crtc_state *cstate, *old_cstate = > to_dpu_crtc_state(crtc->state); > > - old_cstate = to_dpu_crtc_state(crtc->state); > cstate = kmemdup(old_cstate, sizeof(*old_cstate), GFP_KERNEL); > if (!cstate) { > DPU_ERROR("failed to allocate state\n"); > @@ -754,19 +718,12 @@ static struct drm_crtc_state > *dpu_crtc_duplicate_state(struct drm_crtc *crtc) > static void dpu_crtc_disable(struct drm_crtc *crtc, >struct drm_crtc_state *old_crtc_state) > {