Op 03-08-18 om 13:43 schreef Satendra Singh Thakur: > Following changes are done to this func: > 1. The declaration of plane and it's assignment plane = crtc->primary > are only used when mode_valid is set. Therefore, moved it inside > the if(mode_valid) statement. > > 2. The declaration of connector and set_connectors_ptr and out_id > are moved inside the for loop, as their scope is limited within > that block. > > 3. Currently, there are 3 checks on count_connectors > and 4 checks on mode related params (mode_valid, mode, fb). > if (crtc_req->mode_valid) { > if (crtc_req->count_connectors == 0 && mode) { > if (crtc_req->count_connectors > 0 && (!mode || !fb)) { > if (crtc_req->count_connectors > 0) { > > In the modified code, there are just 1 check on mode_valid and > 2 checks count_connectors. > Checks on mode and fb are not needed as these variables will > be non-NULL by the end of if(mode_valid) statement if mode_valid is set. > If mode_valid is clear, mode and fb will be NULL. > Therefore, we just check mode_valid and NOT mode or fb. > > 4. Moved kfree inside if statement > > Signed-off-by: Satendra Singh Thakur <satendra.t@xxxxxxxxxxx> > > --- > > v1: Hi Mr Maarten, Thanks for the comments. > I have fixed some of them and done more modifications to the patch. > Please review. Could you read the suggestions on https://patchwork.freedesktop.org/patch/241508/ ? I'm not saying this code is incorrect, I think that if you want to improve drm then you should tackle something bigger than just function readability. :) Cheers, ~Maarten _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel